Package: emacs;
Reported by: ozzloy <ozzloy <at> gmail.com>
Date: Wed, 7 Jun 2023 06:53:02 UTC
Severity: normal
Tags: patch
View this message in rfc822 format
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: bug#63941: 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!
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.