Issue 3518001: HttpAuthHandlerFactory::CreateDefault requires a HostResolver argument.

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Issue 3518001: HttpAuthHandlerFactory::CreateDefault requires a HostResolver argument. (Closed)

Created:
10 years, 2 months ago by cbentzel

Modified:
9 years, 7 months ago

Reviewers:
eroman

CC:
chromium-reviews, cbentzel+watch_chromium.org, dpranke+watch_chromium.org, ben+cc_chromium.org, amit, pam+watch_chromium.org, Paweł Hajdan Jr., darin-cc_chromium.org

Visibility:
Public.

Description

HttpAuthHandlerFactory::CreateDefault requires a HostResolver argument. Previously the default registry factory would crash if it saw a Negotiate header when trying to do a CNAME lookup because of a NULL resolver. Since then I've changed HttpAuthHandlerNegotiate to skip the CNAME lookup step when the resolver_ is NULL. So, one option is to make CreateDefault accept an optional HostResolver argument. My concern is that since passing in NULL is the easier choice, people will do it and encounter user problems when going through a Negotiate proxy. Hence, I force the correct behavior. BUG=56819 TEST=net_unittests, try fetch client through a Negotiate authenticating proxy. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=60942

Patch Set 1 #

Created: 10 years, 2 months ago

	Unified diffs	Side-by-side diffs	Stats (+53 lines, -24 lines)
M	chrome/browser/net/connection_tester.cc	View	1 chunk	+2 lines, -1 line	0 comments
M	chrome/service/net/service_url_request_context.cc	View	1 chunk	+2 lines, -1 line	0 comments
M	chrome/test/plugin/plugin_test.cpp	View	1 chunk	+2 lines, -1 line	0 comments
M	chrome_frame/test/test_server_test.cc	View	1 chunk	+2 lines, -1 line	0 comments
M	net/http/http_auth_handler_factory.h	View	1 chunk	+5 lines, -1 line	0 comments
M	net/http/http_auth_handler_factory.cc	View	2 chunks	+8 lines, -4 lines	0 comments
M	net/http/http_auth_handler_factory_unittest.cc	View	2 chunks	+3 lines, -1 line	0 comments
M	net/http/http_auth_unittest.cc	View	2 chunks	+3 lines, -1 line	0 comments
M	net/http/http_network_transaction_unittest.cc	View	2 chunks	+4 lines, -2 lines	0 comments
M	net/http/http_proxy_client_socket_pool_unittest.cc	View	2 chunks	+5 lines, -2 lines	0 comments
M	net/socket/ssl_client_socket_pool_unittest.cc	View	2 chunks	+5 lines, -2 lines	0 comments
M	net/spdy/spdy_test_util.h	View	4 chunks	+7 lines, -4 lines	0 comments
M	net/tools/fetch/fetch_client.cc	View	1 chunk	+1 line, -1 line	0 comments
M	net/url_request/url_request_unittest.h	View	1 chunk	+2 lines, -1 line	0 comments
M	webkit/tools/test_shell/test_shell_request_context.cc	View	1 chunk	+2 lines, -1 line	0 comments

Messages

Total messages: 2 (0 generated)

Expand Messages | Collapse Messages

cbentzel

Eric, this is the same CL as before but adds chrome/test/plugin/plugin_test.cpp Unfortunately, that target was ...

10 years, 2 months ago (2010-09-28 19:32:58 UTC) #1

lgtm

Expand Messages | Collapse Messages