763352 - [SPDY] Accessing google homepage through proxy throws download prompt

Status: VERIFIED FIXED

(Core :: Networking: HTTP, defect)

Description

[Grant]

Hi all,

First reported on the 8/6/12 builds (images included):

http://forums.mozillazine.org/viewtopic.php?f=23&t=2484269

Poster reported the error console saying this:

    rror: SyntaxError: illegal character
    Source File: https://apis.google.com/_/apps-static/_/js/gapi/googleapis_client,iframes_styles_bubble_internal/rt=j/ver=nPNdQPXPV58.en./sv=1/am=!tbK8W_8mwqaIodoNDQ/d=1/rs=AItRSTPka8wZmQg3IxPiQbaB0K911EvjbQ
    Line: 1
    Source Code:
    �

    Error: ReferenceError: iframes is not defined
    Source File: https://plusone.google.com/_/+1/fastbutton?bsv=p&url=http%3A%2F%2Fgrooveshark.com%2Falbum%2FExitos%2BCon%2BTradicion%2BSinaloense%2F4311578&size=medium&count=true&expandTo=bottom&annotation=none&hl=en-US&jsh=m%3B%2F_%2Fapps-static%2F_%2Fjs%2Fgapi%2F__features__%2Frt%3Dj%2Fver%3DnPNdQPXPV58.en.%2Fsv%3D1%2Fam%3D!tbK8W_8mwqaIodoNDQ%2Fd%3D1%2Frs%3DAItRSTPka8wZmQg3IxPiQbaB0K911EvjbQ#id=I1_1337865230908&parent=http%3A%2F%2Fgrooveshark.com&rpctoken=818362826&_methods=onPlusOne%2C_ready%2C_close%2C_open%2C_resizeMe%2C_renderstart
    Line: 11

    Error: ReferenceError: gadgets is not defined
    Source File: https://plusone.google.com/_/apps-static/_/js/plusone/p1b,p1p/rt=j/ver=NjiN4Wmo0ZQ.en_US./sv=1/am=!6XIE-74lcjlJP2TtRQ/d=1/rs=AItRSTOigJYNDAsai_VK6ZuY1u5jp_dOgg
    Line: 119

    Error: SyntaxError: illegal character
    Source File: https://mts0.google.com/vt/pt?lyrs=m%40177000000&las=25.672473561951584%3B-100.33212661743164%3B25.726611809740938%3B-100.27204513549805&z=13&ptv=1&callback=_xdc_._0h35zl3c2
    Line: 1
    Source Code:
    �

    Error: no element found
    Source File: https://testpilot.mozillalabs.com/testcases/index.json
    Line: 1


Any ideas?

Comment 1

[Grant]

More info - I only get this when its set up as homepage , but accessing Google after startup it loads fine. My other PC which doesn't use a proxy does not do this. Both have the same addons etc.

Comment 2

[Matthias Versen [:Matti]]

Does it make a difference if you toggle network.http.spdy.enabled to false
Clearing the disk cache may be necessary

Comment 3

[Grant]

Hi Matthias,

Disabling SPDY fixes this, so I guess its SPDY related?

Comment 4

[Loic]

Maybe a regression after SPDY v3 landed (Bug 761775).

Comment 5

[Patrick McManus [:mcmanus]]

grant, I'm out of the office until next week - so patience is appreciated.

can you turn on spdy, but turn off network.http.spdy.enabled.v3 ? That means you will use v2 of spdy and we can see if the issue is specifically the new v3 code.

thanks!

Comment 7

[alex_mayorga]

Attachment: Sample of "fastbutton" save with SPDY v3

I'm the original reporter of this issue.

This is still happening at random for me.

Attached find the some samples of "fastbutton" saves with both SPDY v3 and v2.

Comment 8

[alex_mayorga]

Attachment: Sample of "fastbutton" save with SPDY v2

Comment 9

[alex_mayorga]

Attachment: Last "save" of google.com

This is the latest result of "Saving" google.com as a file.

Comment 10

[Patrick McManus [:mcmanus]]

alex - so you are seeing that with both spdy 2 and spdy 3 (it wasn't clear to me what the attachments are meant to illustrate)?

please also confirm for me whether you are or are not you are using a proxy.

please describe what a "fastbutton" save is. I'm not familiar with that term.

thanks!

Comment 11

[alex_mayorga]

Patrick,

I believe "fastbutton" are those G+ buttons plastered all over the web these days.

I got prompts to "Save file as..." for those with both SPDY 2:
"X-Firefox-Spdy: 2"

and SPDY 3:

"X-Firefox-Spdy: 3"

Nightly is behind a proxy here.

Do let me know anything I could collect to get this resolved.

Comment 12

[Grant]

(In reply to Patrick McManus [:mcmanus] from comment #5)
> grant, I'm out of the office until next week - so patience is appreciated.
> 
> can you turn on spdy, but turn off network.http.spdy.enabled.v3 ? That means
> you will use v2 of spdy and we can see if the issue is specifically the new
> v3 code.
> 
> thanks!

Hi Patrick,

Same for me - disabling and resetting after disabling SPDY v3 doesn't fix this issue. Hope that this helps.

Comment 13

[Patrick McManus [:mcmanus]]

just fyi I'm away on vacation this week - this will get focused attention the week of June 18

Comment 14

[alex_mayorga]

Patrick,

hope you've had a great vacation =)

I first noticed this on 07 Jun 2012 and reported it to mozillaZine Nightly thread[1].

The problem is still present on Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:16.0) Gecko/16.0 Firefox/16.0a1 ID:20120618030531

I can reproduce the "Save file as..." prompts randomly:
 - loading pages with G+ buttons results on prompt to save "fastbutton" (see comment 7 and comment 8)
 - searching for a word on the search bar results on a prompt to save "search" 
 - loading google.com results on a prompt to save a file (see comment 9)

Do let me know if there's anything else I could provide to resolve this.
 
1| http://forums.mozillazine.org/viewtopic.php?p=12043397#p12043397

Comment 15

[Grant]

I noticed when going to Google it will bring the download prompt up - but refreshing it will allow you to browse it, with SPDY still enabled (according to SPDY indicator).

so in short, it seems like first visit is only affected.

Comment 16

[Patrick McManus [:mcmanus]]

Attachment: patch 0

I'm posting this patch as a work-in-progress for your to try out.

Try run is at : https://tbpl.mozilla.org/?tree=Try&rev=afab794cca53 (you can get builds from there, when they're made)

I'm not r?ing this quite yet because I still see a save-as issue in my local tree and I haven't determined if it is related to this bug or not yet.

The problem fixed here is that the server has sent a settings (or ping, or go-away) frame before we realize the SSL/NPN handshake is complete. We use the handshake to determine we are running SPDY. So the server data gets interpreted as straight up HTTP/1 incorrectly. 

Two things make this happen at this time.. 1] normally we won't read any data until we have written a transaction out, but because the proxy requires a CONNECT transaction to setup the tunnel that isn't true in this case.. and 2] until recently the pipeline datastructure was a mediator while NPN was being negotiated - and the pipeline code won't call transaction->writesegments() for any transaction that has not yet been written out in full.

Comment 18

[Patrick McManus [:mcmanus]]

Comment on attachment 634359 [details] [diff] [review]
patch 0

The other save-as issue I mentioned in comment 16 turned out to be unrelated (bug 766159).

Comment 21

[Honza Bambas (:mayhemer)]

Comment on attachment 634359 [details] [diff] [review]
patch 0

Review of attachment 634359 [details] [diff] [review]:
-----------------------------------------------------------------

r=honzab

::: netwerk/protocol/http/nsHttpConnection.cpp
@@ +1366,5 @@
>      bool again = true;
>  
>      do {
> +        if (mCompletedProxyConnect && !mNPNComplete) {
> +            LOG(("NsHttpConnection::OnSocketReadable %p return due to complete "

Change the leading N to small-case.

Please add an explanatory comment like:

"We are preventing reads from the socket by the transaction since the protocol must first be determined by NPN negotiation.  When the target server speaks SPDY, getting here means it most probably has sent SETTINGS frame that the transaction recognizes as HTTP/0.9 response."

@@ +1371,5 @@
> +                 "proxy tunnel but incomplete NPN state\n", this));
> +            rv = NS_OK;
> +            break;
> +        }
> +        

The blank line has white spaces, please remove them.

Comment 22

[Honza Bambas (:mayhemer)]

Comment on attachment 634359 [details] [diff] [review]
patch 0

Review of attachment 634359 [details] [diff] [review]:
-----------------------------------------------------------------

Errr...

Taking back.

Now I recall and realize that I'm experiencing this bug w/o any proxy being set up (in Aurora with SPDYv3 enabled).  The condition should just be if (!mNPNComplete).

Comment 23

[Honza Bambas (:mayhemer)]

Ok, spoke too fast, the condition should be if (!mProxyConnectInProgress && !mNPNComplete)

Comment 24

[Patrick McManus [:mcmanus]]

(In reply to Honza Bambas (:mayhemer) from comment #23)
> Ok, spoke too fast, the condition should be if (!mProxyConnectInProgress &&
> !mNPNComplete)

that's fine. I'm a little surprised the read handler is active without the proxy being used, but your incantation is fine.

Comment 25

[Patrick McManus [:mcmanus]]

Attachment: patch 1

Comment 26

[Honza Bambas (:mayhemer)]

Comment on attachment 635078 [details] [diff] [review]
patch 1

Review of attachment 635078 [details] [diff] [review]:
-----------------------------------------------------------------

r=honzab

::: netwerk/protocol/http/nsHttpConnection.cpp
@@ +1370,5 @@
> +            // from the socket until the results of NPN
> +            // negotiation are known (which is determined from the write path).
> +            // If the server speaks SPDY it is likely the readable data here is
> +            // a spdy settings frame and without NPN it would be misinterpreted
> +            // as HTTP/1

HTTP/0.9 ?

Comment 27

[Patrick McManus [:mcmanus]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/99038d3c6360

Comment 28

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/99038d3c6360

Comment 29

[Patrick McManus [:mcmanus]]

I'll late this bake on m-c until the weekend and then nom for aurora assuming it goes well.

Comment 30

[alex_mayorga]

Patrick,

Thanks for your fix, I no longer get the "Save file as..." prompts on Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:16.0) Gecko/16.0 Firefox/16.0a1 ID:20120621053048

Comment 31

[Patrick McManus [:mcmanus]]

Comment on attachment 635078 [details] [diff] [review]
patch 1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 726366
User impact if declined: common save-as dialog when connecting to spdy service via an http proxy
Testing completed (on m-c, etc.): on m-c for 5 days 
Risk to taking this patch (and alternatives if risky): risk med-low. This is a very heavily exercised path so we would probably know quickly if a problem was caused. alternative would be to disable spdy proxies on FF15
String or UUID changes made by this patch:

Comment 32

[Alex Keybl [:akeybl]]

Comment on attachment 635078 [details] [diff] [review]
patch 1

[Triage Comment]
Since we expect to hear if there is any major fallout here, approving for Aurora 15.

Comment 33

[Patrick McManus [:mcmanus]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/bd3fd0baf45f