humanmade / humanmade/tachyon-plugin

Certain characters in filenames break Tachyon query strings when URL-encoded

Open
#23 1 comment 0 reactions 0 assignees View on GitHub
bug
Dominant language
PHP
Stars
86
Forks
16
PR merge metrics
No merged PRs in 30d

Description

I've noticed that filenames with the "@" character in them are being urlencoded to "%40" in the `tachyon_url()` function, which causes errors when retrieving images from Cloudfront/S3.

This urlencoding was added in #8 as a workaround for a Safari bug in url-encoding non-ASCII characters in URLs. I am not sure whether this bug was resolved or not, as I don't have an example of a URL that was not working previously, but testing locally, I'm able to open images in Safari both with non-ASCII characters in the file name (tested with a file at `/uploads/2018/06/驚くばかり.jpg`) and with "possibly unsafe" characters like @`~.

I'm wondering if it would be safe to revert #8 at this point, so that we can support all filenames that WordPress allows in uploads. (This plugin doesn't seem like the right place to be encoding these filenames anyway, as the Safari bug didn't have anything to do with Tachyon or S3 uploads.)

Examples of filenames that work:

https://leesa-development.aws.hmn.md/tachyon/2018/06/placeholder-feature-image_900x@2x.jpg
https://leesa-development.aws.hmn.md/tachyon/2018/06/placeholder-feature-image_900x@2x.jpg?fit=160,90

And ones that don't as a result of urlencoding:

https://leesa-development.aws.hmn.md/tachyon/2018/06/placeholder-feature-image_900x%402x.jpg
https://leesa-development.aws.hmn.md/tachyon/2018/06/placeholder-feature-image_900x%402x.jpg?fit=160,90

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.