[FFmpeg-devel] [PATCH v3 2/3] avformat/mpjpegdec: fix strict boundary search string

[Moritz Barsnick]

On Tue, Oct 08, 2019 at 11:32:09 +0200, Michael Niedermayer wrote:
> > A side effect is that this coincidentally breaks enforcement of
> > strict MIME headers against servers running motion < 3.4.1, where
> > the boundary announcement in the HTTP headers incorrectly used the
> > prefix "--", which exactly matched this bug's behavior.
>
> Can this be worked around, by for example detecting that case and
> supporting it ?
> Or maybe i misunderstand

It could, but should we really?
- Strict MIME enforcement never really worked upto now, due to a
  programming bug (possibly a misunderstanding of the spec) in ffmpeg.
  This makes me think that hardly anyone ever even used
  -strict_mime_boundary.
- It would now, after this patch, happen to fail with a server which
  implements the same misinterpretation of the spec.

If you want to support both interpretations, you'd need to search for
two different boundaries now. Quite some bloat, if you ask me. I don't
know if it's worth the effort, i.e. whether this will even hit anyone,
as "-strict_mime_boundary" is off by default.

I vote against, but I probably don't decide.

Cheers,
Moritz

--- Long version: ---
To be more clear:
- The HTTP server announces
  Content-type: multipart/x-mixed-replace; boundary="AnyArbitraryBoundaryString"
  It is supposed to use a boundary "\r\n--AnyArbitraryBoundaryString\r\n" in
  the content stream.
- In ffmpeg:
  - we search for a boundary "\r\n--" in standard mode.
  - we search for a boundary "\r\n--AnyArbitraryBoundaryString\r\n" in strict mode.
  - we used to search for a boundary "\r\nAnyArbitraryBoundaryString\r\n" in strict mode (before this fix).

motion < 3.4.1 actually announes:
  Content-type: multipart/x-mixed-replace; boundary="--BoundaryString"
and uses the content separator:
  --BoundaryString
(incorrectly).

This has always worked with ffmpeg's non-strict mode.
This used to work in ffmpeg's strict MIME mode (but incorrectly so).
This no longer works with the fixed ffmpeg strict MIME mode.

Possible change:
ffmpeg parses 'Content-type: multipart/x-mixed-replace; boundary=' as
before.

If the parsed announced Boundary begins with "--", i.e. "--ABCXYZ",
then ffmpeg would check for boundaries:
"--ABCXYZ" and "----ABCXYZ" (the latter being the strictly correct
one).

If the parsed announced Boundary does not begin with "--", i.e. "ABCXYZ",
then ffmpeg would check for boundary:
"--ABCXYZ"