[tbb-commits] [tor-browser/tor-browser-52.6.0esr-8.0-2] Bug 19910: Rip out optimistic data socks handshake variant (#3875)

[gk at torproject.org]

commit 67fa1e520d8cfc420f5bdee4b53b8310df18a977
Author: Kathy Brade <brade at pearlcrescent.com>
Date:   Wed Feb 14 10:39:42 2018 -0500

    Bug 19910: Rip out optimistic data socks handshake variant (#3875)
    
    This has been shown to cause problems with STARTTLS in XMPP (used
    in Tor Messenger) and with Tor Launcher's Moat client. A replacement
    will be added to the Tor daemon itself via bug 5915 (Write patch to
    make socks handshakes succeed instantly).
---
 netwerk/base/nsSocketTransport2.cpp | 44 ++++++++-----------------------------
 netwerk/base/nsSocketTransport2.h   |  4 +---
 netwerk/socket/nsSOCKSIOLayer.cpp   |  3 +--
 3 files changed, 11 insertions(+), 40 deletions(-)

diff --git a/netwerk/base/nsSocketTransport2.cpp b/netwerk/base/nsSocketTransport2.cpp
index 4399465fe534..1bfd1fc91525 100644
--- a/netwerk/base/nsSocketTransport2.cpp
+++ b/netwerk/base/nsSocketTransport2.cpp
@@ -1983,26 +1983,7 @@ nsSocketTransport::OnSocketReady(PRFileDesc *fd, int16_t outFlags)
         // Update poll timeout in case it was changed
         mPollTimeout = mTimeouts[TIMEOUT_READ_WRITE];
     }
-    // Tor 3875: Use optimistic data with SOCKS.
-    // To accomplish this, two new states were added that are only used with
-    // SOCKS connections:
-    //   STATE_SENDINGGET - The SOCKS handshake has proceeded to the
-    //                        "sent connect" state; now it is okay to
-    //                         optimistically send some application data (e.g.,
-    //                         an HTTP GET request).
-    //   STATE_SENTGET -    Optimistic data has been sent; make a second call
-    //                        to PR_ConnectContinue() to allow the SOCKS
-    //                        handshake to finish.
-    else if (mState == STATE_SENDINGGET) {
-        if ((mPollFlags & PR_POLL_WRITE) && (outFlags & ~PR_POLL_READ)) {
-            mOutput.OnSocketReady(NS_OK);  // Allow application data to be sent.
-        }
-        mPollTimeout = mTimeouts[TIMEOUT_READ_WRITE];
-        mPollFlags = (PR_POLL_EXCEPT | PR_POLL_READ);
-        mState = STATE_SENTGET;  // Wait for SOCKS handshake response.
-    }
-    else if (((mState == STATE_CONNECTING) || (mState = STATE_SENTGET)) &&
-             !gIOService->IsNetTearingDown()) {
+    else if ((mState == STATE_CONNECTING) && !gIOService->IsNetTearingDown()) {
         // We do not need to do PR_ConnectContinue when we are already
         // shutting down.
 
@@ -2026,14 +2007,7 @@ nsSocketTransport::OnSocketReady(PRFileDesc *fd, int16_t outFlags)
                 Telemetry::PRCONNECTCONTINUE_BLOCKING_TIME_OFFLINE);
         }
 
-        bool isUsingSocks = mProxyTransparent && !mProxyHost.IsEmpty();
-        if (status == PR_SUCCESS && mState == STATE_CONNECTING) {
-            OnSocketConnected();
-            if (isUsingSocks) {
-                mState = STATE_SENDINGGET;
-            }
-        }
-        else if (status == PR_SUCCESS && mState == STATE_SENTGET) {
+        if (status == PR_SUCCESS) {
             //
             // we are connected!
             //
@@ -2060,17 +2034,17 @@ nsSocketTransport::OnSocketReady(PRFileDesc *fd, int16_t outFlags)
             // If the connect is still not ready, then continue polling...
             //
             if ((PR_WOULD_BLOCK_ERROR == code) || (PR_IN_PROGRESS_ERROR == code)) {
-                if (mState != STATE_SENTGET) {
-                    // Set up the select flags for connect...
-                    mPollFlags = (PR_POLL_EXCEPT | PR_POLL_WRITE);
-                    // Update poll timeout in case it was changed
-                    mPollTimeout = mTimeouts[TIMEOUT_CONNECT];
-                }
+                // Set up the select flags for connect...
+                mPollFlags = (PR_POLL_EXCEPT | PR_POLL_WRITE);
+                // Update poll timeout in case it was changed
+                mPollTimeout = mTimeouts[TIMEOUT_CONNECT];
             }
             //
             // The SOCKS proxy rejected our request. Find out why.
             //
-            else if (PR_UNKNOWN_ERROR == code && isUsingSocks) {
+            else if (PR_UNKNOWN_ERROR == code &&
+                     mProxyTransparent &&
+                     !mProxyHost.IsEmpty()) {
                 code = PR_GetOSError();
                 mCondition = ErrorAccordingToNSPR(code);
             }
diff --git a/netwerk/base/nsSocketTransport2.h b/netwerk/base/nsSocketTransport2.h
index b4baed233ff1..7c85ccdc409f 100644
--- a/netwerk/base/nsSocketTransport2.h
+++ b/netwerk/base/nsSocketTransport2.h
@@ -201,9 +201,7 @@ private:
         STATE_IDLE,
         STATE_RESOLVING,
         STATE_CONNECTING,
-        STATE_TRANSFERRING,
-        STATE_SENDINGGET,
-        STATE_SENTGET
+        STATE_TRANSFERRING
     };
 
     // Safer way to get and automatically release PRFileDesc objects.
diff --git a/netwerk/socket/nsSOCKSIOLayer.cpp b/netwerk/socket/nsSOCKSIOLayer.cpp
index a21dfa4a5a11..5429637c1c3a 100644
--- a/netwerk/socket/nsSOCKSIOLayer.cpp
+++ b/netwerk/socket/nsSOCKSIOLayer.cpp
@@ -83,8 +83,7 @@ public:
     void SetConnectTimeout(PRIntervalTime to);
     PRStatus DoHandshake(PRFileDesc *fd, int16_t oflags = -1);
     int16_t GetPollFlags() const;
-    bool IsConnected() const { return (mState == SOCKS_CONNECTED ||
-                                       mState == SOCKS5_READ_CONNECT_RESPONSE_TOP); }
+    bool IsConnected() const { return mState == SOCKS_CONNECTED; }
     void ForgetFD() { mFD = nullptr; }
     void SetNamedPipeFD(PRFileDesc *fd) { mFD = fd; }