Commit 65aaf056 authored by David Van Cleve's avatar David Van Cleve Committed by Commit Bot

Revert a couple CHECKs on request headers / UA validity

Out of concern about the compat effects on Android WebView if we change
its UA string override API to prevent custom headers, revert a couple
CHECKs added recently to net::HttpRequestHeaders's setters and to
WebContents's "set the UA override" method. (The consequence is we'll go
back to a state where we're regularly silently handling a DCHECK
failure.)

This reverts:
- f998816 "content: Enforce precondition that UA overrides must..."
- e98b84ec "net: Change HttpRequestHeaders::SetHeader's DCHECKs to CHECK"

Alternatives considered:
- extend the network service API to allow privileged clients to append
arbitrary headers to outgoing requests in order to accommodate the
WebView case, migrating the WebView UA override setter to use this (too
much effort)
- move back to a DCHECK on Android only (saddles //net with platform-
specific logic with a compat motivation far removed from the network
stack)
- keep the CHECKs (could have led to WebView-embedding
apps crashing in production with unknown frequency)

Fixed: 1105745
Change-Id: I4423a68f1884da3a9248041f75c800e4803d4e5f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2310774Reviewed-by: default avatarMatt Menke <mmenke@chromium.org>
Reviewed-by: default avatarCharlie Reis <creis@chromium.org>
Reviewed-by: default avatarNate Fischer <ntfschr@chromium.org>
Commit-Queue: David Van Cleve <davidvc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#790652}
parent 111ec941
...@@ -1414,11 +1414,6 @@ void WebContentsImpl::SetUserAgentOverride( ...@@ -1414,11 +1414,6 @@ void WebContentsImpl::SetUserAgentOverride(
if (GetUserAgentOverride() == ua_override) if (GetUserAgentOverride() == ua_override)
return; return;
// This is a CHECK because failing this can be security-relevant; see the
// comment on net::HttpUtil::IsValidHeaderValue.
if (!ua_override.ua_string_override.empty())
CHECK(net::HttpUtil::IsValidHeaderValue(ua_override.ua_string_override));
should_override_user_agent_in_new_tabs_ = override_in_new_tabs; should_override_user_agent_in_new_tabs_ = override_in_new_tabs;
renderer_preferences_.user_agent_override = ua_override; renderer_preferences_.user_agent_override = ua_override;
......
...@@ -386,9 +386,6 @@ class WebContents : public PageNavigator, ...@@ -386,9 +386,6 @@ class WebContents : public PageNavigator,
// renderer initiated, then is-overriding-user-agent is set to true for the // renderer initiated, then is-overriding-user-agent is set to true for the
// NavigationEntry. See SetRendererInitiatedUserAgentOverrideOption() for // NavigationEntry. See SetRendererInitiatedUserAgentOverrideOption() for
// details on how renderer initiated navigations are configured. // details on how renderer initiated navigations are configured.
//
// If nonempty, |ua_override|'s value must not contain '\0', '\r', or '\n' (in
// other words, it must be a valid HTTP header value).
virtual void SetUserAgentOverride(const blink::UserAgentOverride& ua_override, virtual void SetUserAgentOverride(const blink::UserAgentOverride& ua_override,
bool override_in_new_tabs) = 0; bool override_in_new_tabs) = 0;
......
...@@ -108,8 +108,8 @@ void HttpRequestHeaders::SetHeader(const base::StringPiece& key, ...@@ -108,8 +108,8 @@ void HttpRequestHeaders::SetHeader(const base::StringPiece& key,
const base::StringPiece& value) { const base::StringPiece& value) {
// Invalid header names or values could mean clients can attach // Invalid header names or values could mean clients can attach
// browser-internal headers. // browser-internal headers.
CHECK(HttpUtil::IsValidHeaderName(key)) << key; DCHECK(HttpUtil::IsValidHeaderName(key)) << key;
CHECK(HttpUtil::IsValidHeaderValue(value)) << key << ":" << value; DCHECK(HttpUtil::IsValidHeaderValue(value)) << key << ":" << value;
SetHeaderInternal(key, value); SetHeaderInternal(key, value);
} }
...@@ -117,8 +117,8 @@ void HttpRequestHeaders::SetHeaderIfMissing(const base::StringPiece& key, ...@@ -117,8 +117,8 @@ void HttpRequestHeaders::SetHeaderIfMissing(const base::StringPiece& key,
const base::StringPiece& value) { const base::StringPiece& value) {
// Invalid header names or values could mean clients can attach // Invalid header names or values could mean clients can attach
// browser-internal headers. // browser-internal headers.
CHECK(HttpUtil::IsValidHeaderName(key)); DCHECK(HttpUtil::IsValidHeaderName(key));
CHECK(HttpUtil::IsValidHeaderValue(value)); DCHECK(HttpUtil::IsValidHeaderValue(value));
auto it = FindHeader(key); auto 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