bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: ozzloy
Subject: bug#63941: [PATCH] ; always CRLF before non-first boundary in multipart form
Date: Sat, 10 Jun 2023 18:38:44 -0700

> Making the change in mm-url.el sounds scary: that code was written
> many years ago, and who knows where it is used?

Yeah, that makes sense. I'll see what I can find about that, at least
within emacs itself.

> The reason for testing bolp there is not explained, but I'm guessing
> they didn't want to craete an empty line?

Not sure. It looks like it was introduced during a patch intended to fix
submitting binary data.  Would it make sense to CC the two authors of the
following commits?

I dug into the history a bit to try to find out where =(unless (bolp)=
came from

#+begin_src bash
git log -L '/(unless (bolp)/,+1:lisp/gnus/mm-url.el' > unless-bolp
#+end_src

It looks like it was introduced in this commit

#+begin_quote
commit fca2f70380dcb054497470aaf8eda6173063928e
Author: Kenjiro Nakayama <nakayamakenjiro@gmail.com>
Date:   Mon Nov 10 22:33:55 2014 +0100

    Allow uploading files from eww
#+end_quote

and the "\r\n" was included unconditionally before the boundary

#+begin_src elisp
   ;; use the boundary as a separator
   (concat "\r\n--" boundary "\r\n")
#+end_src

Then it this commit changed it

#+begin_quote
commit a6e0188dffc394698d9ffbef50401f14a31c8722
Author: Lars Ingebrigtsen <larsi@gnus.org>
Date:   Thu Oct 13 21:39:29 2016 +0200

    Fix problem with submitting binary data via HTTP forms
#+end_quote

to the following

#+begin_src elisp
 (unless (bolp)
      (insert "\r\n"))
#+end_src

plus a bunch of "\r\n"s scattered throughout the different conditions.
It's not clear to me how the behavior of the function changed.

It looks like maybe instead of the initial diff I proposed, something
like this would produce more accurate output for file uploads, and also
change the surrounding code as little as possible,

#+begin_quote
diff --git a/lisp/gnus/mm-url.el b/lisp/gnus/mm-url.el
index 11847a79f17..686dea20b6a 100644
--- a/lisp/gnus/mm-url.el
+++ b/lisp/gnus/mm-url.el
@@ -430,7 +430,8 @@ mm-url-encode-multipart-form-data
              (insert filedata))
             ;; How can this possibly be useful?
             ((integerp filedata)
-             (insert (number-to-string filedata))))))
+             (insert (number-to-string filedata)))))
+         (insert "\r\n"))
         ((equal name "submit")
          (insert
           "Content-Disposition: form-data; name=\"submit\"\r\n\r\nSubmit\r\n"))
#+end_quote

But it seems like it would be better to do a larger rewrite that
consolidates the places where "\r\n" is added before the non-initial
boundary.

In order to figure that out, I will read the two versions more closely
to figure out what the differences are and write some ERT tests showing
the difference.  Hopefully that will clear things up.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]