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

[ozzloy <ozzloy@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 240 bytes --]

i've modified the commit with a couple goals.
  + make the bug fixing part of the diff as small as possible.
  + made the new tests look more like existing ones.
  + rebased onto the head of the emacs-29 branch pulled in
    earlier today.

[-- Attachment #1.2: Type: text/html, Size: 310 bytes --]

[-- Attachment #2: 0001-upload-newline-terminated-files-via-EWW-Bug-63941.patch --]
[-- Type: text/x-patch, Size: 6327 bytes --]

From c6c42e4a72fc9c26086d7e9f0bcd70999a1bc213 Mon Sep 17 00:00:00 2001
From: Daniel Watson <ozzloy@gmail.com>
Date: Fri, 21 Jul 2023 00:03:06 -0700
Subject: [PATCH] ; upload newline terminated files via EWW (Bug#63941)

; Ensure that every boundary in HTTP message is preceded by
; "\r\n".  According to RFC 2046, section 5, the "\r\n"
; preceding the boundary is not considered part of the
; preceding content, and is instead attached to the boundary
; that follows it.

; Consider a file named "1nl", consisting only of the single
; character '\n'.

; The prior version of =mm-url-encode-multipart-form-data=
; creates the following HTTP message:

;   (concat
;    "--BOUNDARY\r\n"
;    "Content-Disposition: form-data; name=\"a\"; filename=\"1nl\"\r\n"
;    "Content-Transfer-Encoding: binary\r\n"
;    "Content-Type: c\r\n"
;    "\r\n"
;
;    ;; file content
;    "\n"
;
;    ;; NOTE "\r\n" is absent here before the following boundary
;    "--BOUNDARY--\r\n")

; this version of =mm-url-encode-multipart-form-data= creates
; this HTTP message:

;   (concat
;    "--BOUNDARY\r\n"
;    "Content-Disposition: form-data; name=\"a\"; filename=\"1nl\"\r\n"
;    "Content-Transfer-Encoding: binary\r\n"
;    "Content-Type: c\r\n"
;    "\r\n"
;
;    ;; file content
;    "\n"
;
;    ;; NOTE "\r\n" preceding the boundary
;    "\r\n"
;    "--BOUNDARY--\r\n")

; The new code ensures all boundaries after the one at the very
; beginning are preceded by "\r\n", whether they are the final,
; or other internal boundaries.
---
 lisp/gnus/mm-url.el            |   5 +-
 test/lisp/gnus/mm-url-tests.el | 131 +++++++++++++++++++++++++++++++++
 2 files changed, 133 insertions(+), 3 deletions(-)
 create mode 100644 test/lisp/gnus/mm-url-tests.el

diff --git a/lisp/gnus/mm-url.el b/lisp/gnus/mm-url.el
index 11847a79f17..5b68b25ec2e 100644
--- a/lisp/gnus/mm-url.el
+++ b/lisp/gnus/mm-url.el
@@ -433,13 +433,12 @@ mm-url-encode-multipart-form-data
 	      (insert (number-to-string filedata))))))
 	 ((equal name "submit")
 	  (insert
-	   "Content-Disposition: form-data; name=\"submit\"\r\n\r\nSubmit\r\n"))
+	   "Content-Disposition: form-data; name=\"submit\"\r\n\r\nSubmit"))
 	 (t
 	  (insert (format "Content-Disposition: form-data; name=%S\r\n\r\n"
 			  name))
 	  (insert value)))
-	(unless (bolp)
-	  (insert "\r\n"))))
+	(insert "\r\n")))
     (insert "--" boundary "--\r\n")
     (buffer-string)))
 
diff --git a/test/lisp/gnus/mm-url-tests.el b/test/lisp/gnus/mm-url-tests.el
new file mode 100644
index 00000000000..44efba1867c
--- /dev/null
+++ b/test/lisp/gnus/mm-url-tests.el
@@ -0,0 +1,131 @@
+;;; mm-url-tests.el ---  -*- lexical-binding:t -*-
+
+;; Copyright (C) 2021-2023 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;;; Code:
+
+(require 'ert)
+(require 'mm-url)
+
+
+(ert-deftest mm-url-encode-multipart-form-data ()
+  ;; nil
+  (should
+   (string=
+    (mm-url-encode-multipart-form-data '() "BOUNDARY")
+    "--BOUNDARY--\r\n"))
+
+  ;; key value pair
+  (should
+   (string=
+    (mm-url-encode-multipart-form-data
+     '(("key" . "value")) "BOUNDARY")
+    (concat "--BOUNDARY\r\n"
+	    "Content-Disposition: form-data; name=\"key\"\r\n"
+	    "\r\n"
+	    "value\r\n"
+	    "--BOUNDARY--\r\n")))
+
+  ;; submit
+  (should
+   (string=
+    (mm-url-encode-multipart-form-data '(("submit")) "BOUNDARY")
+    (concat "--BOUNDARY\r\n"
+	    "Content-Disposition: form-data; name=\"submit\"\r\n"
+	    "\r\n"
+	    "Submit\r\n"
+	    "--BOUNDARY--\r\n")))
+
+  ;; file ending in newline
+  (should
+   (string=
+    (mm-url-encode-multipart-form-data
+     '(("file" . (("name"         . "a")
+		  ("filename"     . "b")
+		  ("content-type" . "c")
+		  ("filedata"     . "d\n"))))
+     "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"
+     "d\n\r\n"
+     "--BOUNDARY--\r\n")))
+
+  ;; stress test combining parts: key-value, submit, file
+  (should
+   (string=
+    (mm-url-encode-multipart-form-data
+     '(("name" . "value")
+       ("submit")
+       ("file" . (("name"         . "a")
+		  ("filename"     . "b")
+		  ("content-type" . "c")
+		  ("filedata"     . "d"))))
+     "BOUNDARY")
+    (concat
+     "--BOUNDARY\r\n"
+     "Content-Disposition: form-data; name=\"name\"\r\n"
+     "\r\n"
+     "value\r\n"
+     "--BOUNDARY\r\n"
+     "Content-Disposition: form-data; name=\"submit\"\r\n"
+     "\r\n"
+     "Submit\r\n"
+     "--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"
+     "d\r\n"
+     "--BOUNDARY--\r\n")))
+
+  ;; two files, newline at EOF, before final and non-final BOUNDARY
+  (should
+   (string=
+    (mm-url-encode-multipart-form-data
+     '(("file" . (("name"         . "a")
+		  ("filename"     . "b")
+		  ("content-type" . "c")
+		  ("filedata"     . "d\n")))
+       ("file" . (("name"         . "e")
+		  ("filename"     . "f")
+		  ("content-type" . "g")
+		  ("filedata"     . "h\n"))))
+     "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"
+     "d\n\r\n"
+     "--BOUNDARY\r\n"
+     "Content-Disposition: form-data; name=\"e\"; filename=\"f\"\r\n"
+     "Content-Transfer-Encoding: binary\r\n"
+     "Content-Type: g\r\n"
+     "\r\n"
+     "h\n\r\n"
+     "--BOUNDARY--\r\n"))))
+
+
+;;; mm-url-tests.el ends here
-- 
2.39.2