[tor-commits] [tor-browser] 282/311: Bug 1752270 - Retry a request that failed with 408 if HTTP/2 is used. r=necko-reviewers, kershaw a=dmeehan
gitolite role
git at cupani.torproject.org
Tue Apr 26 15:31:22 UTC 2022
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 at 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
+ );
});
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the tor-commits
mailing list