Issue 9595007: Fix for crash in urlmon.dll on telegrafua.com. (Endless redirect loop)

Keyboard Shortcuts

	File
u :	up to issue
m :	publish + mail comments
M :	edit review message
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
<Enter> :	respond to / edit current comment
d :	mark current comment as done

	Issue
u :	up to list of issues
m :	publish + mail comments
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
# :	close issue

	Comment/message editing
<Ctrl> + s or <Ctrl> + Enter :	save comment
<Esc> :	cancel edit

Issue 9595007: Fix for crash in urlmon.dll on telegrafua.com. (Endless redirect loop) (Closed)

Created:
March 5, 2013, 1:41 p.m. by Oleksandr

Modified:
Nov. 12, 2013, 10:01 a.m.

Reviewers:
alex, Felix Dahlke, Wladimir Palant

CC:
Oleksandr

Visibility:
Public.

Description

Fix for crash in urlmon.dll on telegrafua.com. (Endless redirect loop)

Patch Set 1 #

Total comments: 4

Created: March 5, 2013, 1:41 p.m.

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+715 lines, -699 lines)			Patch
	M	Shared/PluginMimeFilterClient.cpp	View		1 chunk	+39 lines, -39 lines	0 comments	Download
	M	Shared/ProtocolImpl.inl	View		1 chunk	+676 lines, -660 lines	4 comments	Download

Messages

Total messages: 3

Expand All Messages | Collapse All Messages

Oleksandr

http://codereview.adblockplus.org/9595007/diff/1/Shared/ProtocolImpl.inl File Shared/ProtocolImpl.inl (left): http://codereview.adblockplus.org/9595007/diff/1/Shared/ProtocolImpl.inl#oldcode559 Shared/ProtocolImpl.inl:559: } The actual fix is below. All other changes ...

March 5, 2013, 1:44 p.m. (2013-03-05 13:44:10 UTC) #1

Wladimir Palant

March 5, 2013, 3:21 p.m. (2013-03-05 15:21:48 UTC) #2

Oleksandr

March 5, 2013, 6:03 p.m. (2013-03-05 18:03:08 UTC) #3

http://codereview.adblockplus.org/9595007/diff/1/Shared/ProtocolImpl.inl
File Shared/ProtocolImpl.inl (right):

http://codereview.adblockplus.org/9595007/diff/1/Shared/ProtocolImpl.inl#newc...
Shared/ProtocolImpl.inl:555: hrResult = S_FALSE;
I have not seen anything like this for any other errors, so I'm assuming it is
INET_E_REDIRECT_FAILED. To me it is really a black box what's going on in
urlmon.dll.

APP stands for HTTP protocol replacement. We get to decide what happens with all
HTTP traffic going through current instance of IE. Luckily most of our
implementation is just a redirection to default HTTP protocol implementation, so
it isn't as big as it sounds. In this case the error occurs in the "default HTTP
implementation", in urlmon.dll in CInet::ReportResultAndStop.

Since this happens even when we are not doing any blocking or any changes
whatsoever I am thinking maybe by default IE has one more layer of abstraction,
which we replace and which handles errors like these before the APP.

On 2013/03/05 15:21:48, Wladimir Palant wrote:
> Is that issue really specific to INET_E_REDIRECT_FAILED? What about all the
> other possible error codes?
> 
> Btw, what does APP stand for? What do we change compared to the case where no
> plugins are installed (and IE supposedly doesn't crash)?

Expand All Messages | Collapse All Messages