On 11/30/15 6:37 AM, wm4 wrote:
On Mon, 30 Nov 2015 08:41:42 +0100
Nicolas George <geo...@nsup.org> wrote:

+                "Expected boundary '%s' not found, instead found a line of %lu 
bytes\n",
+                expected_boundary,
+                strlen(line));
"%lu" is not correct for size_t. The correct type would be %zu, but it is
possible that we have to use another construct to avoid bugs from microsoft
libraries, see other instances in the code for examples.
It's used in some portable code (e.g. http.c), so it's probably fine.

-            size = parse_content_length(value);
-            if (size < 0)
-                return size;
+            *size = parse_content_length(value);
Did you remove the check on purpose?
He probably did, but now that I look at this again, it seems a bit
shady. This code parses the Content-length header, and if parsing it
fails, it will now try the boundary instead. But doesn't this header
always require a numeric value if it's present at all?
It surely does. However, seeing creative ways in which existing endpoints choose to break MIME, I'd rather not introduce a restriction where we don't have to. Take the boundary for example -- the code before my changes (and after as well, at least by default) only looks for "CRLF--". I would've found it unfathomable and unacceptable, if I didn't recently see a trace from a popular consumer camera, where one boundary was declared, and another was used. I'm a huge proponent of standards *compliance*, but when standards *enforcement* will result in a regression report, even if the other side is at fault, choosing to be a bit more flexible may be the way to go. In this particular case I don't see a downside, do you?

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to