Subject: Re: Fwd: bug#63941: [PATCH] ; always CRLF before non-first boundary in multipart form

From: daniel watson <[email-masked]>
Date: Sat, 22 Mar 2025 20:18:16 -0700

> stefan monnier
>>> There remain some questions on this patch:
>>> 1- When `filedata` is neither a number nor a string this is treated
>>> the
>>>    same as an empty string.  Is that right?  Or should it just never
>>>    happen (in which case we should probably catch this case and signal
>>>    an error).

(still have not rediscovered the other 2 bugs i mentioned in my last
message.)

when filedata's value is of type character, it is treated as if it were
an integer.  for all the other datatypes i tested, yes, the function
behaves as if the value is an empty string.

this diff shows calling the function with different filedata values:

https://codeberg.org/ozzloy/emacs/commit/cb8332b37b94e836e3072864594d9bdb1c0ef161
(also available as attached file filedata-value-of-various-types.diff)
diff --git a/test/lisp/gnus/mm-url-tests.el b/test/lisp/gnus/mm-url-tests.el
index 44efba1867c..67628a21006 100644
--- a/test/lisp/gnus/mm-url-tests.el
+++ b/test/lisp/gnus/mm-url-tests.el
@@ -99,6 +99,145 @@ mm-url-encode-multipart-form-data
      "d\r\n"
      "--BOUNDARY--\r\n")))
 
+  ;; file with empty string as filedata
+  (should
+   (string=
+    (mm-url-encode-multipart-form-data
+     '(("file" . (("name" . "a")
+		  ("filename" . "b")
+		  ("content-type" . "c")
+		  ("filedata" . ""))))
+     "BOUNDARY")
+    (concat
+     "--BOUNDARY\r\n"
+     "Content-Disposition: form-data; name=\"a\"; filename=\"b\"\r\n"
+     "Content-Transfer-Encoding: binary\r\n"
+     "Content-Type: c\r\n"
+     "\r\n"
+     "--BOUNDARY--\r\n")))
+
+  ;; test what happens with different types of values for filedata
+  (should
+   ;; filedata's value is of type integer,
+   ;;   integer converted to decimal string representation
+   (string=
+    (mm-url-encode-multipart-form-data
+     '(("file" . (("name" . "a")
+		  ("filename" . "b")
+		  ("content-type" . "c")
+		  ("filedata" . 97))))
+     "BOUNDARY")
+    (concat
+     "--BOUNDARY\r\n"
+     "Content-Disposition: form-data; name=\"a\"; filename=\"b\"\r\n"
+     "Content-Transfer-Encoding: binary\r\n"
+     "Content-Type: c\r\n"
+     "\r\n"
+     "97\r\n"
+     "--BOUNDARY--\r\rn"))
+
+   ;; filedata's value is of type char,
+   ;;   treated same as when filedata is integer ascii code for char
+   (string=
+    (mm-url-encode-multipart-form-data
+     '(("file" . (("name" . "a")
+		  ("filename" . "b")
+		  ("content-type" . "c")
+		  ("filedata" . ?a))))
+     "BOUNDARY")
+    (concat
+     "--BOUNDARY\r\n"
+     "Content-Disposition: form-data; name=\"a\"; filename=\"b\"\r\n"
+     "Content-Transfer-Encoding: binary\r\n"
+     "Content-Type: c\r\n"
+     "97\r\n"
+     "--BOUNDARY--\r\n"))
+
+   ;; filedata's value is of type float,
+   ;;   output is same as when filedata is empty string
+   (string=
+    (mm-url-encode-multipart-form-data
+     '(("file" . (("name" . "a")
+		  ("filename" . "b")
+		  ("content-type" . "c")
+		  ("filedata" . 6.28))))
+     "BOUNDARY")
+    (concat
+     "--BOUNDARY\r\n"
+     "Content-Disposition: form-data; name=\"a\"; filename=\"b\"\r\n"
+     "Content-Transfer-Encoding: binary\r\n"
+     "Content-Type: c\r\n"
+     "\r\n"
+     "--BOUNDARY--\r\n"))
+
+   ;; filedata's value is of type symbol,
+   ;;   output is same as when filedata is empty string
+   (string=
+    (mm-url-encode-multipart-form-data
+     '(("file" . (("name" . "a")
+		  ("filename" . "b")
+		  ("content-type" . "c")
+		  ("filedata" . 'foo))))
+     "BOUNDARY")
+    (concat
+     "--BOUNDARY\r\n"
+     "Content-Disposition: form-data; name=\"a\"; filename=\"b\"\r\n"
+     "Content-Transfer-Encoding: binary\r\n"
+     "Content-Type: c\r\n"
+     "\r\n"
+     "--BOUNDARY--\r\n"))
+
+   ;; filedata's value is of type vector,
+   ;;   output is same as when filedata is empty string
+   (string=
+    (mm-url-encode-multipart-form-data
+     '(("file" . (("name" . "a")
+		  ("filename" . "b")
+		  ("content-type" . "c")
+		  ("filedata" . [6 2 8]))))
+     "BOUNDARY")
+    (concat
+     "--BOUNDARY\r\n"
+     "Content-Disposition: form-data; name=\"a\"; filename=\"b\"\r\n"
+     "Content-Transfer-Encoding: binary\r\n"
+     "Content-Type: c\r\n"
+     "\r\n"
+     "--BOUNDARY--\r\n"))
+
+   ;; filedata's value is of type function,
+   ;;   output is same as when filedata is empty string
+   (string=
+    (mm-url-encode-multipart-form-data
+     '(("file" . (("name" . "a")
+		  ("filename" . "b")
+		  ("content-type" . "c")
+		  ("filedata" . #'car))))
+     "BOUNDARY")
+    (concat
+     "--BOUNDARY\r\n"
+     "Content-Disposition: form-data; name=\"a\"; filename=\"b\"\r\n"
+     "Content-Transfer-Encoding: binary\r\n"
+     "Content-Type: c\r\n"
+     "\r\n"
+     "--BOUNDARY--\r\n"))
+
+   ;; filedata's value is of type symbol,
+   ;;   output is same as when filedata is empty string
+   (string=
+    (mm-url-encode-multipart-form-data
+     '(("file" . (("name" . "a")
+		  ("filename" . "b")
+		  ("content-type" . "c")
+		  ("filedata" . 'foo))))
+     "BOUNDARY")
+    (concat
+     "--BOUNDARY\r\n"
+     "Content-Disposition: form-data; name=\"a\"; filename=\"b\"\r\n"
+     "Content-Transfer-Encoding: binary\r\n"
+     "Content-Type: c\r\n"
+     "\r\n"
+     "--BOUNDARY--\r\n")))
+
   ;; two files, newline at EOF, before final and non-final BOUNDARY
   (should
    (string=
commit cb8332b37b94e836e3072864594d9bdb1c0ef161
Author: daniel watson <[email-masked]>
Date:   Sat Mar 22 20:53:44 2025 -0700

    test different types for filedata to see what they do

what /should/ happen?  not sure.  there's only one place in emacs where
this comes up, lisp/net/eww.el in eww-submit.  in there, "filedata" is
associated with a string.

i did not find any code that calls to mm-url-encode-multipart-form-data
searching on the broader internet.

rfc2046 seems to say it's allowed to be any data that does not
contain the boundary.

from all of that, it seems reasonable to me to only accept string, and
raise deprecation for integer, and signal an error for everything else.

a comment says ";; How can this possibly be useful?" just above the
integer handling code.  so it seems plausible that handling an integer
here is not useful.

let me know if there's something else i can do to help!