bug#36773: 27.0.50; Accessing a cached SVG with eww can cause Emacs to crash

[Pip Cet <pipcet@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1030 bytes --]

On Thu, Jul 25, 2019 at 12:55 PM Eli Zaretskii <eliz@gnu.org> wrote:
> > From: Pip Cet <pipcet@gmail.com>
> > Date: Thu, 25 Jul 2019 11:51:16 +0000
> > Cc: Adam Plaice <plaiceadam@gmail.com>, 36773@debbugs.gnu.org
> >
> > > Perhaps url-store-in-cache should take a parameter to remove the
> > > Content-Encoding header (i.e. "gzip")?  It should really be up to the
> > > program that uses url.el (i.e. shr) whether to cache the data or not...
> >
> > I misread what you wrote at first, but I like my misreading better:
> > url-handle-content-transfer-encoding modifies the message, but not its
> > headers. Why shouldn't it do both?
>
> Yes, I think it should.  Because that's the root cause of the problem:
> the data is uncompressed, but the headers still say it is compressed.

Okay, I think it's likely we're going to require something similar for
other headers, so I added an argument to mail-fetch-field to delete
the fetched field's header line(s).

Patches attached (the first should be unmodified). Appears to work here.

[-- Attachment #2: 0001-Don-t-crash-when-parsing-bad-SVG-data-bug-36773.patch --]
[-- Type: text/x-patch, Size: 1503 bytes --]

From 1b6f3bd532bf1ea819d3780def2e2c9594b1204d Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet@gmail.com>
Date: Wed, 24 Jul 2019 12:34:36 +0000
Subject: [PATCH 1/2] Don't crash when parsing bad SVG data (bug#36773)

* src/image.c (svg_load_image): Be more careful about librsvg
returning NULL pointers.
---
 src/image.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/image.c b/src/image.c
index 355c849491..b1f84e1946 100644
--- a/src/image.c
+++ b/src/image.c
@@ -9530,11 +9530,15 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
   if (base_file)
     g_object_unref (base_file);
   g_object_unref (input_stream);
-  if (err) goto rsvg_error;
+  if (err || rsvg_handle == NULL)
+    goto rsvg_error;
 #else
   /* Make a handle to a new rsvg object.  */
   rsvg_handle = rsvg_handle_new ();
 
+  if (rsvg_handle == NULL)
+    goto rsvg_error;
+
   /* Set base_uri for properly handling referenced images (via 'href').
      See rsvg bug 596114 - "image refs are relative to curdir, not .svg file"
      <https://gitlab.gnome.org/GNOME/librsvg/issues/33>. */
@@ -9654,7 +9658,8 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
   return 1;
 
  rsvg_error:
-  g_object_unref (rsvg_handle);
+  if (rsvg_handle != NULL)
+    g_object_unref (rsvg_handle);
   /* FIXME: Use error->message so the user knows what is the actual
      problem with the image.  */
   image_error ("Error parsing SVG image `%s'", img->spec);
-- 
2.22.0


[-- Attachment #3: 0002-Don-t-double-decompress-cached-HTTP-responses-bug-36.patch --]
[-- Type: text/x-patch, Size: 2907 bytes --]

From 026f799e42c2ca2fa3a3dc1467c8b2ebd5983dd0 Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet@gmail.com>
Date: Thu, 25 Jul 2019 13:22:15 +0000
Subject: [PATCH 2/2] Don't double-decompress cached HTTP responses (bug#36773)

* lisp/url/url-http.el (url-handle-content-transfer-encoding): Modify
the message headers as well as the message body to reflect
decompression.
* lisp/mail/mail-utils.el (mail-fetch-field): Add DELETE argument, to
delete header lines included in the result.
---
 lisp/mail/mail-utils.el | 13 ++++++++++---
 lisp/url/url-http.el    |  2 +-
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/lisp/mail/mail-utils.el b/lisp/mail/mail-utils.el
index cbcbdfaeb2..fd00dd19bc 100644
--- a/lisp/mail/mail-utils.el
+++ b/lisp/mail/mail-utils.el
@@ -284,11 +284,13 @@ 'rmail-dont-reply-to
 
 \f
 ;;;###autoload
-(defun mail-fetch-field (field-name &optional last all list)
+(defun mail-fetch-field (field-name &optional last all list delete)
   "Return the value of the header field whose type is FIELD-NAME.
 If second arg LAST is non-nil, use the last field of type FIELD-NAME.
 If third arg ALL is non-nil, concatenate all such fields with commas between.
 If 4th arg LIST is non-nil, return a list of all such fields.
+If 5th arg DELETE is non-nil, delete all header lines that are
+included in the result.
 The buffer should be narrowed to just the header, else false
 matches may be returned from the message body."
   (save-excursion
@@ -311,7 +313,9 @@ mail-fetch-field
 		  (setq value (concat value
 				      (if (string= value "") "" ", ")
 				      (buffer-substring-no-properties
-				       opoint (point)))))))
+				       opoint (point)))))
+                (if delete
+                    (delete-region (point-at-bol) (point)))))
 	    (if list
 		value
 	      (and (not (string= value "")) value)))
@@ -324,7 +328,10 @@ mail-fetch-field
 		;; Back up over newline, then trailing spaces or tabs
 		(forward-char -1)
 		(skip-chars-backward " \t" opoint)
-		(buffer-substring-no-properties opoint (point)))))))))
+                (prog1
+                    (buffer-substring-no-properties opoint (point))
+                  (if delete
+                      (delete-region (point-at-bol) (1+ (point))))))))))))
 \f
 ;; Parse a list of tokens separated by commas.
 ;; It runs from point to the end of the visible part of the buffer.
diff --git a/lisp/url/url-http.el b/lisp/url/url-http.el
index 527760118d..41bad9dba0 100644
--- a/lisp/url/url-http.el
+++ b/lisp/url/url-http.el
@@ -951,7 +951,7 @@ url-http-parse-headers
                   (start end &optional allow-partial))
 
 (defun url-handle-content-transfer-encoding ()
-  (let ((encoding (mail-fetch-field "content-encoding")))
+  (let ((encoding (mail-fetch-field "content-encoding" nil nil nil t)))
     (when (and encoding
 	       (fboundp 'zlib-available-p)
 	       (zlib-available-p)
-- 
2.22.0