GNU bug report logs - #63941
[PATCH] ; always CRLF before non-first boundary in multipart form

Previous Next

Package: emacs;

Reported by: ozzloy <ozzloy <at> gmail.com>

Date: Wed, 7 Jun 2023 06:53:02 UTC

Severity: normal

Tags: patch

Full log


Message #65 received at 63941 <at> debbugs.gnu.org (full text, mbox):

From: daniel watson <ozzloy <at> each.do>
To: daniel watson <ozzloy <at> each.do>
Cc: 63941 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Stefan Kangas <stefankangas <at> gmail.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: Fwd: bug#63941: [PATCH] ; always CRLF before non-first boundary
 in multipart form
Date: Sat, 22 Mar 2025 20:18:16 -0700
[Message part 1 (text/plain, inline)]
> 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)
[filedata-value-of-various-types.diff (text/x-diff, inline)]
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 <ozzloy <at> each.do>
Date:   Sat Mar 22 20:53:44 2025 -0700

    test different types for filedata to see what they do
[Message part 3 (text/plain, inline)]

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!

This bug report was last modified 91 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.