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

[Pip Cet]

On Wed, Jul 24, 2019 at 10:13 PM Adam Plaice <plaiceadam@gmail.com> wrote:
> > I'm attaching a patch to fix the rsvg segfault, and another patch
> > which works around the url-http issue. However, I'm not sure how the
> > latter should be fixed properly.
>
> Thanks! The first patch indeed prevents the crash, while the second also
> causes the image to be displayed (as expected).

Thank you for testing.

> > -      (zlib-decompress-region (point) (point-max)))))))
> > +      (zlib-decompress-region (point) (point-max) t))))))
>
> > So I guess that is a hint, we could just go back to the Emacs-26
> > behavior. I don't think we should, but in practice it should work
> > okay.
>
> b36913d803ee22a314f2e0a27523fbadeb60dd2c introduced the above change.
> Testing with a checkout of it, results in a blank "standard error box"
> being displayed, though interestingly without a crash.  At
> b36913d803ee22a314f^ the SVG was correctly displayed, so
> b36913d803ee22a314f did indeed introduce (part of) this bug.  However,
> not using ALLOW-PARTIAL, would re-introduce Bug#33133, which would
> probably not be a great idea.

Agreed. As I said, I think it's best to remove the content-encoding
header when interpreting it.

> > I thought that additional argument only mattered upon failure to
> > completely uncompress the data.  Otherwise, the use of that argument
> > should not have changed the behavior.  Are you saying that the
> > decompression failed in this case?  If not, what am I missing?
>
> If I understand the issue correctly, it's because
> `zlib-decompress-region' is trying to decompress content that is in
> the cache and had already been decompressed.

That's my understanding as well.

> Hence, the decompression
> fails and deletes the contents, which, depending on other particulars,
> either crashes Emacs or causes a warning, and in any case prevents the
> actual image from being displayed.

I don't think "allow-partial" properly expresses the "and delete the
specified region unconditionally" semantics we now have. It might make
more sense to replace the region only if at least one byte of data was
successfully decompressed.