Add more comments on how to use Content-Disposition header properly & Fix some trivial problems

actix-http/src/header/common/content_disposition.rs

@@ -76,6 +76,11 @@ pub enum DispositionParam {
     /// the form.
     Name(String),
     /// A plain file name.
+    ///
+    /// It is [not supposed](https://tools.ietf.org/html/rfc6266#appendix-D) to contain any
+    /// non-ASCII characters when used in a *Content-Disposition* HTTP response header, where
+    /// [`FilenameExt`](DispositionParam::FilenameExt) with charset UTF-8 may be used instead
+    /// in case there are Unicode charaters in file names.
     Filename(String),
     /// An extended file name. It must not exist for `ContentType::Formdata` according to
     /// [RFC7578 Section 4.2](https://tools.ietf.org/html/rfc7578#section-4.2).
@@ -220,7 +225,16 @@ impl DispositionParam {
 /// ext-token           = <the characters in token, followed by "*">
 /// ```
 ///
-/// **Note**: filename* [must not](https://tools.ietf.org/html/rfc7578#section-4.2) be used within
+/// # Note
+///
+/// filename is [not supposed](https://tools.ietf.org/html/rfc6266#appendix-D) to contain any
+/// non-ASCII characters when used in a *Content-Disposition* HTTP response header, where
+/// filename* with charset UTF-8 may be used instead in case there are Unicode characters in file
+/// names.
+/// filename is [acceptable](https://tools.ietf.org/html/rfc7578#section-4.2) to be UTF-8 encoded
+/// directly in a *Content-Disposition* header for *multipart/form-data*, though.
+///
+/// filename* [must not](https://tools.ietf.org/html/rfc7578#section-4.2) be used within
 /// *multipart/form-data*.
 ///
 /// # Example
@@ -251,6 +265,22 @@ impl DispositionParam {
 /// };
 /// assert_eq!(cd2.get_name(), Some("file")); // field name
 /// assert_eq!(cd2.get_filename(), Some("bill.odt"));
+///
+/// // HTTP response header with Unicode charaters in file names
+/// let cd3 = ContentDisposition {
+///     disposition: DispositionType::Attachment,
+///     parameters: vec![
+///         DispositionParam::FilenameExt(ExtendedValue {
+///             charset: Charset::Ext(String::from("UTF-8")),
+///             language_tag: None,
+///             value: String::from("\u{1f600}.svg").into_bytes(),
+///         }),
+///         // fallback for better compatibility
+///         DispositionParam::Filename(String::from("Grinning-Face-Emoji.svg"))
+///     ],
+/// };
+/// assert_eq!(cd3.get_filename_ext().map(|ev| ev.value.as_ref()),
+///            Some("\u{1f600}.svg".as_bytes()));
 /// ```
 ///
 /// # WARN
@@ -342,6 +372,7 @@ impl ContentDisposition {
                 let param = if param_name.eq_ignore_ascii_case("name") {
                     DispositionParam::Name(value)
                 } else if param_name.eq_ignore_ascii_case("filename") {
+                    // See also comments in test_from_raw_uncessary_percent_decode.
                     DispositionParam::Filename(value)
                 } else {
                     DispositionParam::Unknown(param_name.to_owned(), value)
@@ -466,11 +497,40 @@ impl fmt::Display for DispositionType {
 
 impl fmt::Display for DispositionParam {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        // All ASCII control charaters (0-30, 127) excepting horizontal tab, double quote, and
+        // All ASCII control charaters (0-30, 127) including horizontal tab, double quote, and
         // backslash should be escaped in quoted-string (i.e. "foobar").
-        // Ref: RFC6266 S4.1 -> RFC2616 S2.2; RFC 7578 S4.2 -> RFC2183 S2 -> ... .
+        // Ref: RFC6266 S4.1 -> RFC2616 S3.6
+        // filename-parm  = "filename" "=" value
+        // value          = token | quoted-string
+        // quoted-string  = ( <"> *(qdtext | quoted-pair ) <"> )
+        // qdtext         = <any TEXT except <">>
+        // quoted-pair    = "\" CHAR
+        // TEXT           = <any OCTET except CTLs,
+        //                  but including LWS>
+        // LWS            = [CRLF] 1*( SP | HT )
+        // OCTET          = <any 8-bit sequence of data>
+        // CHAR           = <any US-ASCII character (octets 0 - 127)>
+        // CTL            = <any US-ASCII control character
+        //                  (octets 0 - 31) and DEL (127)>
+        //
+        // Ref: RFC7578 S4.2 -> RFC2183 S2 -> RFC2045 S5.1
+        // parameter := attribute "=" value
+        // attribute := token
+        //              ; Matching of attributes
+        //              ; is ALWAYS case-insensitive.
+        // value := token / quoted-string
+        // token := 1*<any (US-ASCII) CHAR except SPACE, CTLs,
+        //             or tspecials>
+        // tspecials :=  "(" / ")" / "<" / ">" / "@" /
+        //               "," / ";" / ":" / "\" / <">
+        //               "/" / "[" / "]" / "?" / "="
+        //               ; Must be in quoted-string,
+        //               ; to use within parameter values
+        //
+        //
+        // See also comments in test_from_raw_uncessary_percent_decode.
         lazy_static! {
-            static ref RE: Regex = Regex::new("[\x01-\x08\x10\x1F\x7F\"\\\\]").unwrap();
+            static ref RE: Regex = Regex::new("[\x00-\x08\x10-\x1F\x7F\"\\\\]").unwrap();
         }
         match self {
             DispositionParam::Name(ref value) => write!(f, "name={}", value),
@@ -774,8 +834,18 @@ mod tests {
 
     #[test]
     fn test_from_raw_uncessary_percent_decode() {
+        // In fact, RFC7578 (multipart/form-data) Section 2 and 4.2 suggests that filename with
+        // non-ASCII characters MAY be percent-encoded.
+        // To the contrary, RFC6266 or other RFCs related to Content-Disposition response header
+        // do not mention such percent-encoding.
+        // So, it appears to be undecidable whether to percent-decode or not without
+        // knowning the usage scenario (multipart/form-data v.s. HTTP response header) and
+        // inevitable to unnecessarily percent-decode filename with %XX in the former scenario.
+        // Fortunately, it seems that almost all mainstream browsers just send UTF-8 encoded file
+        // names in quoted-string format (tested on Edge, IE11, Chrome and Firefox) without
+        // percent-encoding. So we do not bother to attempt to percent-decode.
         let a = HeaderValue::from_static(
-            "form-data; name=photo; filename=\"%74%65%73%74%2e%70%6e%67\"", // Should not be decoded!
+            "form-data; name=photo; filename=\"%74%65%73%74%2e%70%6e%67\"",
         );
         let a: ContentDisposition = ContentDisposition::from_raw(&a).unwrap();
         let b = ContentDisposition {