New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 596579 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 596523



Sign in to add a comment

QuicSpdyClientStream::OnInitialHeadersComplete() will accept a :status header that contains a leading plus sign.

Project Member Reported by eroman@chromium.org, Mar 21 2016

Issue description

Is that correct?

It uses base::StringToInt() without checking for a leading plus. I expect a leading plus sign is invalid here.
 

Comment 1 by eroman@chromium.org, Mar 21 2016

Components: -Internals>Network Internals>Network>QUIC

Comment 2 by rch@chromium.org, Mar 21 2016

Owner: zhongyi@chromium.org
Status: Assigned (was: Untriaged)
zhongyi, can you take a look at this when you have some time?
Cc: rjshade@chromium.org
The following change added this code.

Internal change: 108694474

https://codereview.chromium.org/1501493003/

https://codereview.chromium.org/1501493003/diff/1/net/tools/quic/quic_spdy_client_stream.cc
Sure. Getting hands on this. 
Both internal and chromium code takes the leading plus as valid, considering it as the positive number. 

Will fix the leading plus check in the internal first, then merge it to chromium to fix it here. 

Comment 6 by rch@chromium.org, Mar 22 2016

It's possible that the internal version does not have the same issue. I'm not sure.
I had tested the internal code, it accepts the leading plus unfortunately :(
Status: Started (was: Assigned)
The bug is fixed in internal code now. We have added strict checking to only acknowledge 3-digit-integer status in the range of [100, 599]. Once the code is merged to chromium, I will give a update here and mark this bug as fixed. 
Status: Fixed (was: Started)
Mark this issue as fixed as internal code has been landed to chromium. FYI, QuicSpdyStream::ParseHeaderStatusCode now will do a strict 3-digit status field checking in header.  

Sign in to add a comment