Skip to content
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

ContentDisposition.toString() should include both regular and extended filename parameter #29861

Closed
arjohn-telecats opened this issue Jan 20, 2023 · 6 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: feedback-provided Feedback has been provided type: enhancement A general enhancement
Milestone

Comments

@arjohn-telecats
Copy link

The Content-Disposition header generated by ContentDisposition.toString() includes either the regular filename parameter or the extended filename* parameter, depending on the value of the charset attribute. For backwards compatibilities with older browser it would be preferable to always include the regular parameter, in addition to the extended parameter. This also seems to be what RFC 6266 suggests:

RFC 6266, section 4.3:
Many user agent implementations predating this specification do not understand the "filename*" parameter. Therefore, when both "filename" and "filename*" are present in a single header field value, recipients SHOULD pick "filename*" and ignore "filename". This way, senders can avoid special-casing specific user agents by sending both the more expressive "filename*" parameter, and the "filename" parameter as fallback for legacy recipients (see [Section 5] for an example).

RFC 6266, section 5 (Examples):
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

Note: Those user agents that do not support the [RFC 5987] encoding ignore "filename*" when it occurs after "filename".

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 20, 2023
@poutsma
Copy link
Contributor

poutsma commented Jan 23, 2023

While having both parameters sounds like a good idea, I am having difficulty seeing how we can transform any given String into an US-ASCII string for the filename parameter. For instance, given the filename € rates, I don't see how we can transliterate into EUR, as seen in the example.

@poutsma poutsma added status: waiting-for-feedback We need additional information before we can continue in: web Issues in web modules (web, webmvc, webflux, websocket) labels Jan 23, 2023
@arjohn-telecats
Copy link
Author

arjohn-telecats commented Jan 23, 2023

Transliterating this is way out of scope for the ContentDisposition class imho. Options that might work for the regular attribute:

  1. Simply url-encode any non-ascii characters (using utf8?) and hope that the browser decodes it in the same way.
  2. Replace any non-ascii characters with a replacement character, an underscore for example.
  3. Extended the API and allow developers to set both attributes separately.

The first option works for me with the latest Chrome and Firefox releases.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jan 23, 2023
@poutsma poutsma self-assigned this Jan 31, 2023
@poutsma poutsma added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 31, 2023
@poutsma poutsma added this to the 6.0.5 milestone Jan 31, 2023
@poutsma poutsma closed this as completed in 58fbf60 Feb 1, 2023
@poutsma
Copy link
Contributor

poutsma commented Feb 1, 2023

I decided to resolve this by encoding any non-ASCII characters as quoted-printable (RFC 2047), as that seems to be the most common way to do so. Moreover, we already had decoding support for it.

@arjohn-telecats
Copy link
Author

RFC 2047 is related to MIME/email. Are you sure that this also applies to HTTP? I've never seen a quoted printable HTTP header, nor was I able to find any references to this on the internet. URL-encoding seems to be the commonly used method for filename parameters. Perhaps a quick test to check how a browser treats quoted-printable encoding might be in order.

@poutsma
Copy link
Contributor

poutsma commented Feb 2, 2023

As a general remark, non-ASCII filename encodings in Content-Disposition are a mess. Or at least they were, before RFC 5987.

That said, RFC 2047 does apply filenames in HTTP Content-Disposition fields. It is supported by Commons File Upload, Firefox, and Chrome. Granted, browsers are looking to drop support in favor of RFC 5987, but it's still supported, also by Spring Framework.

In Content-Disposition::parse, we already support RFC 2047, 5987, and Base64 filenames. So it made sense to use either quoted printable or base 64 for encoding the filename parameter, otherwise we had to introduce a fourth parsing mechanism as well, or the result of toString would not be parsable by parse. I decided to use RFC 2047 because its results are more readable than Base64.

mdeinum pushed a commit to mdeinum/spring-framework that referenced this issue Jun 29, 2023
This commit ensures the ContentDisposition class prints the filename in
both in the regular filename parameter and the extended filename*
parameter (RFC 5987).
Quoted printable (RFC 2047) is used to encode any non-ASCII characters
in the regular filename parameter.

Closes spring-projectsgh-29861
@mariuszpala

This comment was marked as duplicate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: feedback-provided Feedback has been provided type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants