Skip to content
This repository has been archived by the owner on Jun 11, 2024. It is now read-only.

Commit

Permalink
Properly escape uploaded image names
Browse files Browse the repository at this point in the history
As reported in #156, we weren't escaping image names containing plus
signs.

URI.escape('a + b') will produce
'a%20+%20b'
which is treated by the browser as 'a   b'.

We need something that escapes plus signs as well, just like
encodeURIComponent does on the javascript side of things. I'm surprised
there's nothing better than having to pass a regexp to URI.escape in
Ruby, but that seems to be how things are done [1].

We need tests around this, but I'll punt on that until we have things
converted to js.

[1] http://stackoverflow.com/questions/2834034/how-do-i-raw-url-encode-decode-in-javascript-and-ruby-to-get-the-same-values-in-b
  • Loading branch information
trotzig committed Sep 23, 2016
1 parent 811cb62 commit f03b5be
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 1 deletion.
6 changes: 6 additions & 0 deletions example-project/example.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ happo.define('hallooo', function() {
document.body.appendChild(elem);
}, { viewports: ['mobile', 'desktop'] });

happo.define('hallooo + something else', function() {
var elem = document.createElement('span');
elem.innerHTML = 'Hioyi!<br>Hello';
document.body.appendChild(elem);
}, { viewports: ['mobile', 'desktop'] });

happo.define('bar', function() {
var elem = document.createElement('span');
elem.innerHTML = 'go bars!<br>bars';
Expand Down
4 changes: 3 additions & 1 deletion lib/happo/uploader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,9 @@ def upload_image(example, variant)
"#{variant}.png"))
image.content_type = 'image/png'
image.save
URI.escape(img_name)

# http://stackoverflow.com/questions/2834034/how-do-i-raw-url-encode-decode-in-javascript-and-ruby-to-get-the-same-values-in-b
URI.escape(img_name, Regexp.new("[^#{URI::PATTERN::UNRESERVED}]"))
end
end
end

2 comments on commit f03b5be

@lencioni
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be worth writing a pending spec, so that we don't forget to test it later.

@lencioni
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, thanks for fixing this!

Please sign in to comment.