Skip to content

fix non-ASCII filnemae garbled on Internet Explorer #7

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 23, 2020

Conversation

djmonta
Copy link
Contributor

@djmonta djmonta commented Jan 23, 2020

Issue

Filename include any non-ASCII characters corrupt on Internet Explorer.

Description

  • Set header Content-Dispoistion based on RFC 6266 (RFC 2231/RFC 5987) specification.
  • Use rawurlencode to convert the filename.

@mikehaertl
Copy link
Owner

Thanks, this makes sense.

I just want to make sure that older browsers are still supported. Some suggest to add both filename= and filename*=utf-8 as mentioned in the example section of that RFC:

This example is the same as the one above, but adding the "filename" parameter for compatibility with user agents not implementing RFC 5987:

 Content-Disposition: attachment;
                      filename="EURO rates";
                      filename*=utf-8''%e2%82%ac%20rates

Everything should still be in 1 line, though.

Both parameters are also listed here:

The parameters "filename" and "filename*" differ only in that "filename*" uses the encoding defined in RFC 5987. When both "filename" and "filename*" are present in a single header field value, "filename*" is preferred over "filename" when both are understood.

What do you think?

@djmonta
Copy link
Contributor Author

djmonta commented Jan 23, 2020

Thank you for suggestion.
I fixed the code leaving filename parameter as you wrote, supporting older browsers.

@mikehaertl mikehaertl merged commit 8690e61 into mikehaertl:master Jan 23, 2020
@mikehaertl
Copy link
Owner

mikehaertl commented Jan 23, 2020

Great, thanks. Just released 1.1.2 containing this fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants