As requested in SPR-14408, this PR adds a method to get the filename
parameter of the Content-Disposition
header.
@bclozel I have not yet included support for filename*
encoded headers, because I am not sure we should support this. See this comment for more details. I will not merge this PR before your feedback.
Comment From: sdeleuze
After more reading, filename*
is likely to be the right way to go for modern browsers, so I am going to update this PR with support for decoding it.
Comment From: sdeleuze
@rstoyanchev @poutsma I think this PR is ready to be reviewed by one of you before going to master.
Comment From: sdeleuze
@rstoyanchev I have pushed an updated commit with HttpHeaders#getContentDisposition()
. I have implemented the support for the disposition type and name
, filename
, filename*
and size
parameters.
If we don't have date parsing according to RFC 822 already implemented, I would suggest to wait an eventual user request to implement creation-date
, modification-date
and read-date
support (my guess is that they are not very commonly used, and in any case this is outside of the scope of the original issue).
I have keep an array based parsing instead of the Stream
based one you proposed because Java 8 streams seems not really design to collect successive pairs + I think current implementation will be faster.
I have made the 2 encode and decode methods private as requested.
Could you have another look to this updated PR and send me your feedback if any?
Comment From: rstoyanchev
Sorry for the delay. A few last tweaks. Let's make ContentDisposition a top-level type like HttpMethod, MediaType, HttpRange. The parse logic can then be a static method in Content-Disposition following the same pattern as HttpRange. Similarly the setContentDisposition should take a ContentDisposition and the actual logic to format to a String can be in ContentDisposition, again like HttpRange. In other words let's encapsulate all the Content-Disposition related logic inside ContentDisposition
and HttpHeaders will simply delegate to it.
Comment From: sdeleuze
No worry, thanks for your feedback, I will update the PR accordingly.
Comment From: rstoyanchev
Okay, feel free to commit straight to master after those changes.
Comment From: sdeleuze
Merged via https://github.com/spring-projects/spring-framework/commit/99a8510ace46af9b05b822e7c65f08aae885ca98