Commit 6c2cdfb8 authored by ricea's avatar ricea Committed by Commit bot

Revert of Make calling SetHeader() with invalid value fatal (patchset #1 id:1...

Revert of Make calling SetHeader() with invalid value fatal (patchset #1 id:1 of https://codereview.chromium.org/2143903002/ )

Reason for revert:
This CL has served its purpose of finding bad callers of SetHeader() and should be reverted before the branch point.

Original issue's description:
> Make calling SetHeader() with invalid value fatal
>
> crrev.com/2134083003 made net::HttpUtil::IsValidHeaderValue() reject
> individual CR and NL as well as CRNL.
>
> I believe that all callers of net::HttpRequestHeaders::SetHeader() and
> SetHeaderIfMissing() which use user-supplied values already verify the
> value with IsValidHeaderValue() first. However, to be sure, temporarily
> make it a fatal error to call SetHeader() with an invalid value.
>
> If you see a crash attributed to this CL:
>
> 1. Associate it with the bug.
> 2. Follow the stack flow to work out how untrusted data ended up
> being passed to SetHeader().
> 3. Add a call to IsValidHeaderValue() at the point where the untrusted
> data was introduced. A tighter check such as IsToken() may be appopriate.
>
> BUG=627398
>
> Committed: https://crrev.com/c5c1a790acc423f359c22641ac2ab0f3f9e6d7a9
> Cr-Commit-Position: refs/heads/master@{#405702}

R=mmenke@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=627398

Review-Url: https://codereview.chromium.org/2268423004
Cr-Commit-Position: refs/heads/master@{#414633}
parent eae115a5
...@@ -90,8 +90,7 @@ void HttpRequestHeaders::Clear() { ...@@ -90,8 +90,7 @@ void HttpRequestHeaders::Clear() {
void HttpRequestHeaders::SetHeader(const base::StringPiece& key, void HttpRequestHeaders::SetHeader(const base::StringPiece& key,
const base::StringPiece& value) { const base::StringPiece& value) {
DCHECK(HttpUtil::IsValidHeaderName(key.as_string())); DCHECK(HttpUtil::IsValidHeaderName(key.as_string()));
// TODO(ricea): Revert this. See crbug.com/627398. DCHECK(HttpUtil::IsValidHeaderValue(value.as_string()));
CHECK(HttpUtil::IsValidHeaderValue(value.as_string()));
HeaderVector::iterator it = FindHeader(key); HeaderVector::iterator it = FindHeader(key);
if (it != headers_.end()) if (it != headers_.end())
it->value.assign(value.data(), value.size()); it->value.assign(value.data(), value.size());
...@@ -102,8 +101,7 @@ void HttpRequestHeaders::SetHeader(const base::StringPiece& key, ...@@ -102,8 +101,7 @@ void HttpRequestHeaders::SetHeader(const base::StringPiece& key,
void HttpRequestHeaders::SetHeaderIfMissing(const base::StringPiece& key, void HttpRequestHeaders::SetHeaderIfMissing(const base::StringPiece& key,
const base::StringPiece& value) { const base::StringPiece& value) {
DCHECK(HttpUtil::IsValidHeaderName(key.as_string())); DCHECK(HttpUtil::IsValidHeaderName(key.as_string()));
// TODO(ricea): Revert this. See crbug.com/627398. DCHECK(HttpUtil::IsValidHeaderValue(value.as_string()));
CHECK(HttpUtil::IsValidHeaderValue(value.as_string()));
HeaderVector::iterator it = FindHeader(key); HeaderVector::iterator it = FindHeader(key);
if (it == headers_.end()) if (it == headers_.end())
headers_.push_back(HeaderKeyValuePair(key, value)); headers_.push_back(HeaderKeyValuePair(key, value));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment