URLs with encoded characters not handled properly. about kv-asset-handler HOT 7 CLOSED

ooohhh - very interesting.

I've become so conditioned to respond to URL encoding issues with a recommendation to polyfill that I didn't actually read your report.

Your diagnosis seems correct and using decodeURI should fix 👍

Happy to accept a PR or we can add this to our list of things to address.

from kv-asset-handler.

Hey @computator - this is actually an issue with the Workers runtime itself (very longstanding). In our docs we recommend polyfilling the URL class with a better implementation if you need to. Let me know if you need any help figuring that out 😄

from kv-asset-handler.

The runtime URL class may be nonconformant, but the percent encoding aspect of it is: https://url.spec.whatwg.org/#example-5434421b. If you test it in chrome or something you can see it returns the pathname percent encoded as well:

> new URL('http://google.com/test%20path').pathname
'/test%20path'

As such, it is in fact the kv-asset-handler code that is not compensating for that encoding before loading files from the KV store. 😁

from kv-asset-handler.

Any update on this?

When using filenames in non-latin scripts, like Cyrillic or Arabic, kv-asset-handler will also look for the encoded name instead of the decoded name.

Using decodeURI fixes this. But it is futile to use decodeURI in mapRequestToAsset, because Request() encodes the URL right back. So decodeURI needs to be run in kv-asset-handler itself.

Since some people may use encoded filenames, I think there should be an option to decode the path name.

from kv-asset-handler.

I think this is a little insane. If you have a CMS (which most sites probably do), and you allow editors to upload images, for example (which also, you probably do), you're bound to have some image filenames with a space in them. And spaces in a filename does not work!

This broke our production site yesterday. I would consider this package core to the Worker sites offering, so [Cloudflare] please don't hide behind the "yeah, just fix it yourself and open a PR if you care about this" argument. Personally I think that wrangler should encode the filenames, but I don't know your [Cloudflare] development plans.

from kv-asset-handler.

I think the issue could be closed now. My PR #100 & #105 have fixed the issue.

cc @cloudflare

from kv-asset-handler.

This will be fixed shortly in 0.0.11.

from kv-asset-handler.