Last Comment Bug 766929 - NS_ABORT_IF_FALSE(mConnectionInfo->UsingSSL()) failing [websockets http proxy]
: NS_ABORT_IF_FALSE(mConnectionInfo->UsingSSL()) failing [websockets http proxy]
Status: RESOLVED FIXED
[http-conn]
:
Product: Core
Classification: Components
Component: Networking: HTTP (show other bugs)
: Trunk
: x86_64 Windows 7
: -- normal (vote)
: mozilla16
Assigned To: Jason Duell [:jduell] (needinfo? me)
:
Mentors:
Depends on: 713026
Blocks: 767516
  Show dependency treegraph
 
Reported: 2012-06-21 05:50 PDT by Honza Bambas (:mayhemer)
Modified: 2012-07-10 15:50 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix abort. (1.30 KB, patch)
2012-06-22 11:47 PDT, Jason Duell [:jduell] (needinfo? me)
mcmanus: review+
Details | Diff | Review

Description Honza Bambas (:mayhemer) 2012-06-21 05:50:31 PDT
Trying to reproduce bug 766817 with http://www.websocket.org/echo.html.  Using squid configured for authentication (standard config).  When clicking the [connect] button, I'm getting assertion failure at http://hg.mozilla.org/mozilla-central/annotate/10e019421e6b/netwerk/protocol/http/nsHttpChannel.cpp#l1002

    NS_ABORT_IF_FALSE(mConnectionInfo->UsingSSL(),
                      "SSL connect failed but not using SSL?");

Http status is 403, host is echo.websocket.org, port 80.  mSSLConnectFailed is set to 'true' on the transaction.

Candidate for bug 715905?
Comment 1 Patrick McManus [:mcmanus] 2012-06-21 06:09:34 PDT
it might be as simple as the ABORT being superfulous. iirc websockets uses connect to make a websockets tunnel through the proxy.. That path is the failed-to-make-tunnel path.. off the top of my head the error handling is going to be the same even in the absence of SSL.. but it obviously needs more rigor than that.
Comment 2 Jason Duell [:jduell] (needinfo? me) 2012-06-22 11:47:48 PDT
Created attachment 635841 [details] [diff] [review]
fix abort.

Patrick's comment is right about the issue.  I grepped through the code to audit all uses of "SSL" and found a few other bugs, and a lot of stuff that should get renamed.  Filing separate bugs for those so we can sort out which deserve aurora/beta, if any.  Not sure if this one does, for instance (doesn't affect release builds).
Comment 3 Jason Duell [:jduell] (needinfo? me) 2012-06-22 12:11:31 PDT
Whoops--since this depends on bug 713026 and that isn't making it to aurora/beta, it's moot to consider backporting this as well.
Comment 4 Patrick McManus [:mcmanus] 2012-06-25 05:59:32 PDT
Comment on attachment 635841 [details] [diff] [review]
fix abort.

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

the assert reason should be updated.. and maybe the general comments for the function too - I wouldn't go too crazy chasing the rabbit down the hole here though.
Comment 5 Jason Duell [:jduell] (needinfo? me) 2012-06-25 12:51:13 PDT
Comments for the assertion and the function generally got updated in bug 767516. 

https://hg.mozilla.org/integration/mozilla-inbound/rev/4eca26d02c84
Comment 6 Ed Morley [:emorley] 2012-06-25 13:41:53 PDT
Push backed out for M5 orange:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=d45ca69b67a3

Couldn't see you on #developers or see any try run URLs listed in-bug in the four bugs in the push, or I would have tried to find out if just part of it could have been backed out.

https://hg.mozilla.org/integration/mozilla-inbound/rev/51cff2123f45
Comment 7 Honza Bambas (:mayhemer) 2012-07-02 09:52:03 PDT
Jason, could we land just this one single patch?
Comment 8 Jason Duell [:jduell] (needinfo? me) 2012-07-09 13:01:04 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/9b12977096b9
Comment 9 Ryan VanderMeulen [:RyanVM] 2012-07-10 15:50:19 PDT
https://hg.mozilla.org/mozilla-central/rev/9b12977096b9

Note You need to log in before you can comment on or make changes to this bug.