BUGFIX: Fix loading of resources with spaces in filenames

[sebids]

Encode filenames loaded from S3 to prevent problems with browsers that don't fully support spaces in URIs.

We encountered an error when using this package. When you have a file like "Screenshot 17 of 2015-09-21.png" it works great if you use it in a src-Attribute of an HTML img element.

Problem is when you use the srcset-Attribute. It has a format like this: "Image1@2x.jpg 2x, Image.jpg 1x"

So the image names are not delimited by quotes or something like that. With the filename from the example it would look like that: "Screenshot 17 of 2015-09-21@2x.png 2x, Screenshot 17 of 2015-09-21.png 1x"

So Safari now thinks that the image is named "Screenshot" and it is not found, obviously.

My simple solution is to rawurlencode the filename so that it does not cause problems with the srcset.

What do you think?

[bwaidelich]

According to the HTML spec, the src attribute of the img tag () needs to be a "valid non-empty URL potentially surrounded by spaces" so the fix makes sense in general. But IMO it should not be the filename that is encoded but rather the URL that is returned. So probably we should rather encode it in getPublicPersistentResourceUri() or maybe even in the ResourceManager itself if possible.

[robertlemke]

@bwaidelich if we did that, couldn't that end up in double encoding? Because, we don't know for sure what a custom resource storage / publishing target will return, or?

[bwaidelich]

@robertlemke sorry, didn't see your response earlier. If all implementations do it correctly (don't change the filename, but encode the URIs) there should be no risk

[bwaidelich]

:+1: by reading (and Sebastian tested this)

[kitsunet]

I already tried earlier to enforce uri encoding at a low level in the framework, it's the only responsible place really....

[bwaidelich]

@kitsunet I'm not sure whether I got you. it is the only responsible place to do it in the framework (and if yes, where exactly?) or like sebastian did in this PR? So is it a +1 or a -1?

[kdambekalns]

Ping…

[kdambekalns]

Hey @bwaidelich – @kitsunet – @robertlemke – any news on this one?

[bwaidelich]

no news from me, I still stick to what I wrote in Sept 2015 ;)

[kdambekalns]

Doesn't this solve #15 as well?

[kitsunet]

The "good" solution would probably be to never return strings for URIs but always Uri objects UNTIL it is really embedded in another string and then the URI object takes care of correct escaping/encoding.

[kdambekalns]

Fellow contributors… how do we proceed here?

[kdambekalns]

TBH I don't know if this actually still is an issue, but this change is really old by now. Feel free to re-open and resolve conflicts, if you like! 🎄