URLRequestJob::NotifyHeadersComplete() permits a content-length header that contains a leading + |
|||||||
Issue descriptionBy virtue of using base::StringToInt64() and not checking for a leading plus. Moreover, it does not check for failure from StringToInt64() return value.
,
Mar 21 2016
eroman: I'm assuming you're not planning on taking these on? Thinking of adding a bug to reference all of these, and tossing it into a fixit (Which I'm actually planning on holding next quarter).
,
Mar 21 2016
I believe that is covered by the blocking bug (issue 596523). If you agree with https://bugs.chromium.org/p/chromium/issues/detail?id=596523#c7 then I can handle these. If not we can kick it to the fixit bucket.
,
Mar 21 2016
Oops... Used to the old trackers double messages on linking bug dependencies, and didn't see the second message for that. I agree with issue 596523.
,
Mar 23 2016
,
Jul 6
,
Jul 6
,
Aug 1
,
Aug 1
I'll poke at this - look at other browsers first, then make the change.
,
Aug 2
Actually...we have like 25 places that set content.length, and a grand total of 2 that actually use it. Maybe we can just remove it? The download system does use it, I guess (Which matters for blobs and data URLs, at least), but I'll see how things look without it.
,
Aug 2
URLFetcher uses it in download progress events - I think we may be able to manage without it in SimpleURLLoader, though, so going to put off any removal until then. Have a patch to fix the + issue, at least, which I will send out for review, later today.
,
Aug 6
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ff4871b346b7188f232c4c346d58a4d5a4c07c93 commit ff4871b346b7188f232c4c346d58a4d5a4c07c93 Author: Matt Menke <mmenke@chromium.org> Date: Mon Aug 06 19:33:22 2018 URLRequestJob allowed leading +'s when getting expected content size Not only are leading +'s not allowed in the Content-Length according to spec, our normal Content-Length parsing code doesn't allow them either. This CL switches to using the HttpResponseHeaders Content-Length parser, to match HttpStreamParser's behavior. Unclear if this makes any sense for H2/QUIC, or if we should be doing something else entirely. Bug: 596582 Change-Id: I0520ade19f7eaa50cb9f4aab60338a19e97a1bd4 Reviewed-on: https://chromium-review.googlesource.com/1161250 Commit-Queue: Matt Menke <mmenke@chromium.org> Reviewed-by: Bence Béky <bnc@chromium.org> Cr-Commit-Position: refs/heads/master@{#580950} [modify] https://crrev.com/ff4871b346b7188f232c4c346d58a4d5a4c07c93/net/url_request/url_request_job.cc [modify] https://crrev.com/ff4871b346b7188f232c4c346d58a4d5a4c07c93/net/url_request/url_request_job_unittest.cc
,
Aug 6
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by eroman@chromium.org
, Mar 21 2016