191739 - Conn: not work if loopback is disabled

Status: VERIFIED FIXED

(Core :: Networking: HTTP, defect, P1)

Description

[Mike Kaply [:mkaply]]

As a result of 190000, the browser doesn't crash anymore but still doesn't work
at all.

It gives the error "http is not a registered protocol"

Getting the protocol handler is failing in IOService.

This only happens if loopback is disabled.

I'm still investigating.

Comment 1

[Mike Kaply [:mkaply]]

when loopback is disabled, querying the nsIProxiedProtocolHandler interface fails:

http://lxr.mozilla.org/seamonkey/source/netwerk/base/src/nsIOService.cpp#455

Comment 2

[Mike Kaply [:mkaply]]

Did there used to be a backup to pollable events if they didn't work?

This code in NSPR:

http://lxr.mozilla.org/seamonkey/source/nsprpub/pr/src/io/prsocket.c#1504

PR_InitializeNetAddr(PR_IpAddrLoopback, 0, &selfAddr); /* BugZilla: 35408 */
if (PR_Bind(listenSock, &selfAddr) == PR_FAILURE) {
  goto failed;

checks for loopback and fails if it is not there.

This is actually why mEventThread was null in 190000.

So we have a MAJOR regression in the networking code that it doesn't support
loopback.

And the reason this happens on older Windows as well is because you used to be
able to disable loopback.

Comment 3

[David G King]

If nsIProxiedProtocolHandler fails, then whatever is calling it (i.e. calls
nsIOService::NewChannelFromURI) should have code to handle a fail/NULL return code.

Do you know what is calling this?

Comment 4

[Mike Kaply [:mkaply]]

http://lxr.mozilla.org/seamonkey/source/netwerk/base/public/nsNetUtil.h#163

which was called from:

http://lxr.mozilla.org/seamonkey/source/docshell/base/nsDocShell.cpp#5227

which was called from:

http://lxr.mozilla.org/seamonkey/source/docshell/base/nsDocShell.cpp#5145

which was called from:

http://lxr.mozilla.org/seamonkey/source/docshell/base/nsDocShell.cpp#714

It's the very first http load in the browser.

Comment 5

[Darin Fisher]

ok, the solution of the old socket transport was to set a small timeout on
PR_Poll and basically just busy wait.  i had mistakenly thought that that was
only required for Mac classic running with an older version of NSPR.  i'll put
together a similar patch for the new socket transport ASAP.

Comment 6

[Darin Fisher]

Attachment: v1 patch

this should do the trick.

Comment 7

[Mike Kaply [:mkaply]]

I can confirm that this fixes the problem. Thanks darin!

Comment 8

[Darin Fisher]

bz said he would be able to review this tomorrow.  hopefully that is not too
late for 1.3 beta.

Comment 9

[Doug Turner (:dougt)]

Comment on attachment 113438 [details] [diff] [review]
v1 patch

r=dougt

mike says it works.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 113438 [details] [diff] [review]
v1 patch

>+nsSocketTransportService::ServiceEventQ()
>+{
>+    PRBool result;

How about giving this a name that makes it clear what it does?	Like
"continueProcessing"?


>+        if (!mThreadEvent)
>+            NS_WARNING("running socket transport thread without a pollable event");

Replace that by NS_WARN_IF_FALSE(mThreadEvent, "running ....");  -- one less
branch at runtime.  ;)

>+    // returns FALSE to stop processing the main loop
>+    PRBool ServiceEventQ();

You mean PR_FALSE, right?

>+    PRInt32  Poll();

Brief comment as to what this does?

Comment 11

[Darin Fisher]

Comment on attachment 113438 [details] [diff] [review]
v1 patch

seeking drivers approval for 1.3 beta.	this patch only looks big ;-)  ... no
seriously, most of the code changes are to a path that is never taken by most
users.

Comment 12

[Asa Dotzler [:asa]]

Comment on attachment 113438 [details] [diff] [review]
v1 patch

This needs to land today if it's  going to make 1.3. 
a=asa (on behalf of drivers) for checkin to 1.3beta.

Comment 13

[Darin Fisher]

fixed-on-trunk

Comment 14

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

bug 190000 is still fixed (some other way), right?

Comment 15

[David G King]

If I remember correctly, Bug 190000 was fixed via a NULL check on mThreadEvent,
so I assume it's still fixed. You've changed the fix, but just by changing the
returned message. Oh, by the way, did I see "if (!mThreadEvent)" within a "if
(!mThreadEvent)" in your patch?

Comment 16

[Max Alekseyev]

Today's build still has the problem ;(

Comment 17

[Mike Kaply [:mkaply]]

Of course it does. He didn't check in until 1 in the afternoon.

It will be in tomorrows build.

Comment 18

[benc]

Who disables loopback? I can make this a testcase, but I'd need to know why.

Comment 19

[Darin Fisher]

ben: apparently it is fairly common on OS/2.  mkaply verified the patch, see
comment #7.

Comment 20

[Mike Kaply [:mkaply]]

My understanding is that some proxy servers and firewalls do this as well (on
Windows)

Comment 21

[David G King]

To add to comment #20.

I just installed the latest nightly on WinXP, on launching Mozilla, ZoneAlarm (a
software firewall) asked if I wanted to allow Mozilla to access IP address
127.0.0.1. If I said no, and this patch wasn't in, then Mozilla would have
crashed either on startup or later on.

Comment 22

[benc]

VERIFIED per mike's comment.

I've received some criticism for filing bugs about necko's standards compliance
that are too essoteric in some people's minds, but running w/o a loopback hard
for me to contemplate.

Right now, I'm focused on basic unit testing and getting the verification list
down. If I ever get to local firewall testing, I'll add this. David, what
happens now that this is fixed? If there are any problems post-fix, please file
a new bug and note it here.