Escape Content-Disposition params according to WHATWG HTML living standard

[mkurz]

...TBA...
So, what is this pull request about:
Currently, when Play needs to build a Content-Disposition: form-data; name="..."; filename="..." header, e.g. for sending mulitpart/form data via ws or when when using the RequestBuilder, it does not escape the name or filename. That can cause problems, if a user-entered filename contains "malicious" letters. I wont go into details here, but it's clear that could cause security issues. However IMHO the impact shouldn't be to high in real world apps, because if a file gets uploaded to Play, Play will provide the file name escaped, retrieved from the multi part form body. If the file now gets forwarded to a backend service via ws it will likely be forwarded with the name escaped anyway. So to really make use of the vulnerability you need to set the name of the file from an external source e.g. from user input or from a database entry, etc. Still this can happen, so it's good to get that fixed ;)
The patches I apply here follow the same approach that other frameworks use:

• The Symfony PHP framework behaves exactly the same
• urlllib3 also uses the html5 approach
• go escapes the fields with backslashes
• Spring only allows certain characters, otherwise it throws exceptions.

Offtopic:
What I find really funny, that because of the HTML5 strategy of escaping (file) names of "content-dispostion: form-data" headers, it is not possible for a server to find out if a user uploaded a file name named hello" %22, because the client (be it chrome, be it firefox or curl), will always encode " to %22, so the name gets transfered as hello%22 %22, so when the server decodes the file name again it will think the file was called hello" ", so it's impossible to correctly transfer all information 😉
After reading some RFC's and the HMLT5 standard regarging this encoding stuff, I don't know why the clients did now stick to the *= approach described in RFC5987, that Play e.g. uses for serving files.

---

Some links, which I am just collecting here in case I have to come back to this issue and which helped me along the way:

• urllib3 switching to the html5 approach:
  • Consider HTML 5 draft for multipart/form-data urllib3/urllib3#303
  • HTML5 non-ASCII File Names by Default urllib3/urllib3#856
  • Change fields.py to use HTML5 non-ASCII File Names by Default instead of RFC2231 for encoding urllib3/urllib3#1492
• Links to related RFC's (I bookmarked them a while ago, not sure if all are relevant, but too lazy to go through it once more)
  • https://www.rfc-editor.org/rfc/rfc5987#section-4.2
  • https://datatracker.ietf.org/doc/html/rfc8187
  • https://datatracker.ietf.org/doc/html/rfc2231 (obsolets https://www.rfc-editor.org/rfc/rfc2184)
    • Fixes #6275 Support rfc2184 extended attribute in multipart disposition #8783
  • https://www.rfc-editor.org/rfc/rfc6266#section-4.3 (till section 5)
  • https://www.rfc-editor.org/rfc/rfc7578#section-2
• various related issues in the psf request library:
  • File name encoding cannot be set when uploading files psf/requests#4218
  • Can't use post to upload a file with Chinese characters in its name. psf/requests#2313
  • utf-8 chars in name of file in post requests psf/requests#5224
• Some more
  • https://stackoverflow.com/questions/93551/how-to-encode-the-filename-parameter-of-content-disposition-header-in-http
  • https://bugzilla.mozilla.org/show_bug.cgi?id=1556711
  • Specify escaping in the multipart/form-data encoding algorithm whatwg/html#6282
  • Specify HTML numeric character reference fallback encoding for filenames whatwg/html#3276
  • Spring
    • spring-projects/spring-framework@0583b33
    • Escape quotes in filename in ContentDisposition.Builder when charset not specified spring-projects/spring-framework#24220

[mkurz]

This is the main part of this pull request, everything else just make use of this method and the rest is testing.
The link the the "WHATWG HTML living standard" section 4.10.21.8: https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#multipart-form-data

[mkurz]

I updated the issues description now. Hopefully it's understandable, but just look at the code, I think it's pretty straight-forward. Just to make it clear, this pr is about when Play does generate the header, e.g. when it sends a request to some other service via ws. Parsing a multiplart/form data request from a client does work correctly, so most uses cases should be fine anyway.

I am going to merge this since I worked on that a while ago where I was sure this is done, and now reviewed again carefully and I am very sure this is ok. Also actually this is an issue someone e-mailed me about and that person came up with a similiar patch (but that person did not handle all uses cases, only the main one), so that also confirms my approach here.

[mkurz]

@Mergifyio backport 2.8.x

[mergify]

backport 2.8.x

✅ Backports have been created

• #11603 [2.8.x] Escape Content-Disposition params according to WHATWG HTML living standard (backport #11571) by @mkurz has been created for branch 2.8.x

[motoyasu-saburi]

@mkurz Glad to see that it has been corrected successfully 👏

Content-Disposition filename escaping problem occurs in all Requests/Responses/Emails, and I have compiled a report on this issue.
I have put together a report on this issue.

https://gist.github.com/motoyasu-saburi/1b19ef18e96776fe90ba1b9f910fa714

I'm also very happy to be able to contribute to the Playframework in some small way.
Thank you @mkurz !

[mkurz]

@motoyasu-saburi Thanks for your detailed report and proposed patch back then which helped to track down the problem!