Galooshi / happo

Visual diffing in CI for user interfaces
504 stars 16 forks source link

Broken when `+` character is in the description #192

Open yzimet opened 7 years ago

yzimet commented 7 years ago

If the description contains a + character (maybe others too?), the resulting image is broken:

image

trotzig commented 7 years ago

Hi @yzimet, and thanks for the report! Judging by the screenshot, it looks like you are running a fairly old version of happo. I tried to reproduce this with the latest version, but it looks like we correctly escape characters at this point.

Can you give a later version a try?

lencioni commented 7 years ago

This is from Happo 4.0.1, which I think is the latest version.

lencioni commented 7 years ago

When I looked at it, it seemed like the URL was correctly encoded on the page, but the asset didn't exist. I wonder if there is something weird with the uploader.

trotzig commented 7 years ago

Ah, I rushed to conclusions. I looked at the screenshot and didn't see the tabs above the image, thinking it was an old version. But I now realize that screenshot is probably for a new image, correct?

We're not doing much on our end to escape things. We basically pass in a Key and then use the Location that's sent back in the response from aws. Relevant code: https://github.com/Galooshi/happo/blob/c291f5fa9e729e49c8b1d2246a288da18ed51052/packages/happo-uploader-s3/src/S3Uploader.js#L102