This is an automated email from the git hooks/post-receive script.
pierov pushed a commit to branch geckoview-99.0.1-11.0-1 in repository tor-browser.
commit dc9b23fa08e053087c1ddaf75254d86775568696 Author: Dragana Damjanovic dd.mozilla@gmail.com AuthorDate: Tue Mar 22 21:12:21 2022 +0000
Bug 1752270 - Retry a request that failed with 408 if HTTP/2 is used. r=necko-reviewers,kershaw a=dmeehan
Differential Revision: https://phabricator.services.mozilla.com/D140827 --- netwerk/protocol/http/ConnectionHandle.cpp | 4 +++ netwerk/protocol/http/Http2Session.cpp | 7 ++-- netwerk/protocol/http/Http3Session.cpp | 2 ++ netwerk/protocol/http/Http3Session.h | 1 - netwerk/protocol/http/HttpConnectionBase.h | 4 ++- netwerk/protocol/http/HttpConnectionUDP.cpp | 4 +++ netwerk/protocol/http/nsAHttpConnection.h | 4 ++- netwerk/protocol/http/nsHttpConnection.cpp | 13 ++------ netwerk/protocol/http/nsHttpTransaction.cpp | 21 ++++++++++++ netwerk/test/unit/test_http_408_retry.js | 51 ++++++++++++++++++++++------- 10 files changed, 83 insertions(+), 28 deletions(-)
diff --git a/netwerk/protocol/http/ConnectionHandle.cpp b/netwerk/protocol/http/ConnectionHandle.cpp index 20986e0d3cc8a..180d479492020 100644 --- a/netwerk/protocol/http/ConnectionHandle.cpp +++ b/netwerk/protocol/http/ConnectionHandle.cpp @@ -84,5 +84,9 @@ void ConnectionHandle::TopBrowsingContextIdChanged(uint64_t id) { // Do nothing. }
+PRIntervalTime ConnectionHandle::LastWriteTime() { + return mConn->LastWriteTime(); +} + } // namespace net } // namespace mozilla diff --git a/netwerk/protocol/http/Http2Session.cpp b/netwerk/protocol/http/Http2Session.cpp index 78297e36385aa..fd748d21a0d16 100644 --- a/netwerk/protocol/http/Http2Session.cpp +++ b/netwerk/protocol/http/Http2Session.cpp @@ -4430,8 +4430,7 @@ nsresult Http2Session::OnHeadersAvailable(nsAHttpTransaction* transaction, nsHttpRequestHead* requestHead, nsHttpResponseHead* responseHead, bool* reset) { - return mConnection->OnHeadersAvailable(transaction, requestHead, responseHead, - reset); + return NS_OK; }
bool Http2Session::IsReused() { @@ -4671,5 +4670,9 @@ bool Http2Session::CanAcceptWebsocket() { (mPeerAllowsWebsockets || !mProcessedWaitingWebsockets); }
+PRIntervalTime Http2Session::LastWriteTime() { + return mConnection->LastWriteTime(); +} + } // namespace net } // namespace mozilla diff --git a/netwerk/protocol/http/Http3Session.cpp b/netwerk/protocol/http/Http3Session.cpp index 40f5c885930f0..c2174c8ebee3a 100644 --- a/netwerk/protocol/http/Http3Session.cpp +++ b/netwerk/protocol/http/Http3Session.cpp @@ -1896,5 +1896,7 @@ nsresult Http3Session::GetTransactionSecurityInfo(nsISupports** secinfo) { return NS_OK; }
+PRIntervalTime Http3Session::LastWriteTime() { return mLastWriteTime; } + } // namespace net } // namespace mozilla diff --git a/netwerk/protocol/http/Http3Session.h b/netwerk/protocol/http/Http3Session.h index 9d98c515e9b17..b2265c3e1facf 100644 --- a/netwerk/protocol/http/Http3Session.h +++ b/netwerk/protocol/http/Http3Session.h @@ -95,7 +95,6 @@ class Http3Session final : public nsAHttpTransaction, public nsAHttpConnection {
int64_t GetBytesWritten() { return mTotalBytesWritten; } int64_t BytesRead() { return mTotalBytesRead; } - PRIntervalTime LastWriteTime() { return mLastWriteTime; }
nsresult SendData(nsIUDPSocket* socket); nsresult RecvData(nsIUDPSocket* socket); diff --git a/netwerk/protocol/http/HttpConnectionBase.h b/netwerk/protocol/http/HttpConnectionBase.h index 6235b7af5e92c..34dd394f5d8ad 100644 --- a/netwerk/protocol/http/HttpConnectionBase.h +++ b/netwerk/protocol/http/HttpConnectionBase.h @@ -136,6 +136,7 @@ class HttpConnectionBase : public nsSupportsWeakReference { virtual nsresult GetPeerAddr(NetAddr* addr) = 0; virtual bool ResolvedByTRR() = 0; virtual bool GetEchConfigUsed() = 0; + virtual PRIntervalTime LastWriteTime() = 0;
protected: // The capabailities associated with the most recent transaction @@ -189,7 +190,8 @@ NS_DEFINE_STATIC_IID_ACCESSOR(HttpConnectionBase, HTTPCONNECTIONBASE_IID) bool IsReused() override; \ [[nodiscard]] nsresult PushBack(const char* data, uint32_t length) override; \ void SetEvent(nsresult aStatus) override; \ - virtual nsAHttpTransaction* Transaction() override; + virtual nsAHttpTransaction* Transaction() override; \ + PRIntervalTime LastWriteTime() override;
} // namespace net } // namespace mozilla diff --git a/netwerk/protocol/http/HttpConnectionUDP.cpp b/netwerk/protocol/http/HttpConnectionUDP.cpp index 8fe5f7e9b0c3f..2a2155f9a8722 100644 --- a/netwerk/protocol/http/HttpConnectionUDP.cpp +++ b/netwerk/protocol/http/HttpConnectionUDP.cpp @@ -448,6 +448,10 @@ nsresult HttpConnectionUDP::ForceSend() {
HttpVersion HttpConnectionUDP::Version() { return HttpVersion::v3_0; }
+PRIntervalTime HttpConnectionUDP::LastWriteTime() { + return mHttp3Session->LastWriteTime(); +} + //----------------------------------------------------------------------------- // HttpConnectionUDP <private> //----------------------------------------------------------------------------- diff --git a/netwerk/protocol/http/nsAHttpConnection.h b/netwerk/protocol/http/nsAHttpConnection.h index f1e47c8baa64c..ed595105a4ac3 100644 --- a/netwerk/protocol/http/nsAHttpConnection.h +++ b/netwerk/protocol/http/nsAHttpConnection.h @@ -164,6 +164,7 @@ class nsAHttpConnection : public nsISupports { virtual nsresult GetPeerAddr(NetAddr* addr) = 0; virtual bool ResolvedByTRR() = 0; virtual bool GetEchConfigUsed() = 0; + virtual PRIntervalTime LastWriteTime() = 0; };
NS_DEFINE_STATIC_IID_ACCESSOR(nsAHttpConnection, NS_AHTTPCONNECTION_IID) @@ -257,7 +258,8 @@ NS_DEFINE_STATIC_IID_ACCESSOR(nsAHttpConnection, NS_AHTTPCONNECTION_IID) } \ bool GetEchConfigUsed() override { \ return (!(fwdObject)) ? false : (fwdObject)->GetEchConfigUsed(); \ - } + } \ + PRIntervalTime LastWriteTime() override;
// ThrottleResponse deliberately ommited since we want different implementation // for h1 and h2 connections. diff --git a/netwerk/protocol/http/nsHttpConnection.cpp b/netwerk/protocol/http/nsHttpConnection.cpp index 8b01fad1a12bf..cb879aefe87ae 100644 --- a/netwerk/protocol/http/nsHttpConnection.cpp +++ b/netwerk/protocol/http/nsHttpConnection.cpp @@ -1134,18 +1134,7 @@ nsresult nsHttpConnection::OnHeadersAvailable(nsAHttpTransaction* trans,
// deal with 408 Server Timeouts uint16_t responseStatus = responseHead->Status(); - static const PRIntervalTime k1000ms = PR_MillisecondsToInterval(1000); if (responseStatus == 408) { - // If this error could be due to a persistent connection reuse then - // we pass an error code of NS_ERROR_NET_RESET to - // trigger the transaction 'restart' mechanism. We tell it to reset its - // response headers so that it will be ready to receive the new response. - if (mIsReused && ((PR_IntervalNow() - mLastWriteTime) < k1000ms)) { - Close(NS_ERROR_NET_RESET); - *reset = true; - return NS_OK; - } - // timeouts that are not caused by persistent connection reuse should // not be retried for browser compatibility reasons. bug 907800. The // server driven close is implicit in the 408. @@ -1621,6 +1610,8 @@ HttpVersion nsHttpConnection::Version() { return mLastHttpResponseVersion; }
+PRIntervalTime nsHttpConnection::LastWriteTime() { return mLastWriteTime; } + //----------------------------------------------------------------------------- // nsHttpConnection <private> //----------------------------------------------------------------------------- diff --git a/netwerk/protocol/http/nsHttpTransaction.cpp b/netwerk/protocol/http/nsHttpTransaction.cpp index 1fc862a704ec2..3fc80715143c5 100644 --- a/netwerk/protocol/http/nsHttpTransaction.cpp +++ b/netwerk/protocol/http/nsHttpTransaction.cpp @@ -2169,6 +2169,27 @@ nsresult nsHttpTransaction::HandleContentStart() { mNoContent = true; LOG(("this response should not contain a body.\n")); break; + case 408: + LOG(("408 Server Timeouts")); + + if (mConnection->Version() >= HttpVersion::v2_0) { + mForceRestart = true; + return NS_ERROR_NET_RESET; + } + + // If this error could be due to a persistent connection + // reuse then we pass an error code of NS_ERROR_NET_RESET + // to trigger the transaction 'restart' mechanism. We + // tell it to reset its response headers so that it will + // be ready to receive the new response. + LOG(("408 Server Timeouts now=%d lastWrite=%d", PR_IntervalNow(), + mConnection->LastWriteTime())); + if ((PR_IntervalNow() - mConnection->LastWriteTime()) >= + PR_MillisecondsToInterval(1000)) { + mForceRestart = true; + return NS_ERROR_NET_RESET; + } + break; case 421: LOG(("Misdirected Request.\n")); gHttpHandler->ClearHostMapping(mConnInfo); diff --git a/netwerk/test/unit/test_http_408_retry.js b/netwerk/test/unit/test_http_408_retry.js index 421d3a52665e9..872107ed85391 100644 --- a/netwerk/test/unit/test_http_408_retry.js +++ b/netwerk/test/unit/test_http_408_retry.js @@ -27,17 +27,41 @@ add_task(async function test() { async function check408retry(server) { info(`Testing ${server.constructor.name}`); await server.execute(`global.server_name = "${server.constructor.name}";`); - await server.registerPathHandler("/test", (req, resp) => { - let oldSock = global.socket; - global.socket = resp.socket; - if (global.socket == oldSock) { - resp.writeHead(408); - resp.end("stuff"); - return; - } - resp.writeHead(200); - resp.end(global.server_name); - }); + if ( + server.constructor.name == "NodeHTTPServer" || + server.constructor.name == "NodeHTTPSServer" + ) { + await server.registerPathHandler("/test", (req, resp) => { + let oldSock = global.socket; + global.socket = resp.socket; + if (global.socket == oldSock) { + setTimeout( + arg => { + arg.writeHead(408); + arg.end("stuff"); + }, + 1100, + resp + ); + return; + } + resp.writeHead(200); + resp.end(global.server_name); + }); + } else { + await server.registerPathHandler("/test", (req, resp) => { + let oldSock = global.socket; + global.socket = resp.socket; + if (!global.sent408) { + global.sent408 = true; + resp.writeHead(408); + resp.end("stuff"); + return; + } + resp.writeHead(200); + resp.end(global.server_name); + }); + }
async function load() { let { req, buff } = await loadURL( @@ -59,5 +83,8 @@ add_task(async function test() { await load(); }
- await with_node_servers([NodeHTTPServer, NodeHTTPSServer], check408retry); + await with_node_servers( + [NodeHTTPServer, NodeHTTPSServer, NodeHTTP2Server], + check408retry + ); });