bug 597684 Implement HTTP Assoc-req and Banned Pipelines on nsHttpConnectionInfo r=honzab

--HG--
extra : rebase_source : 26243e1af9e0554553520dae435cc4dcfd551746
This commit is contained in:
Patrick McManus 2012-03-20 13:11:32 -04:00
parent 0fb95ce9a6
commit 92aaf5d2fc
17 changed files with 328 additions and 12 deletions

View File

@ -811,6 +811,10 @@ pref("network.http.pipelining.maxrequests" , 4);
// Prompt for 307 redirects
pref("network.http.prompt-temp-redirect", true);
// If true generate CORRUPTED_CONTENT errors for entities that
// contain an invalid Assoc-Req response header
pref("network.http.assoc-req.enforce", false);
// On networks deploying QoS, it is recommended that these be lockpref()'d,
// since inappropriate marking can easily overwhelm bandwidth reservations
// for certain services (i.e. EF for VoIP, AF4x for interactive video,

View File

@ -2159,6 +2159,24 @@ SpdySession::PipelineDepthAvailable()
return 0;
}
nsresult
SpdySession::SetPipelinePosition(PRInt32 position)
{
// This API is meant for pipelining, SpdySession's should be
// extended with AddStream()
NS_ABORT_IF_FALSE(false,
"SpdySession::SetPipelinePosition() should not be called");
return NS_ERROR_NOT_IMPLEMENTED;
}
PRInt32
SpdySession::PipelinePosition()
{
return 0;
}
//-----------------------------------------------------------------------------
// Pass through methods of nsAHttpConnection
//-----------------------------------------------------------------------------

View File

@ -123,6 +123,12 @@ public:
// called to count the number of sub transactions that can be added
virtual PRUint16 PipelineDepthAvailable() = 0;
// Used to inform the connection that it is being used in a pipelined
// context. That may influence the handling of some errors.
// The value is the pipeline position.
virtual nsresult SetPipelinePosition(PRInt32) = 0;
virtual PRInt32 PipelinePosition() = 0;
};
#define NS_DECL_NSAHTTPTRANSACTION \
@ -144,7 +150,9 @@ public:
PRUint32 Http1xTransactionCount(); \
nsresult TakeSubTransactions(nsTArray<nsRefPtr<nsAHttpTransaction> > &outTransactions); \
nsresult AddTransaction(nsAHttpTransaction *); \
PRUint16 PipelineDepthAvailable();
PRUint16 PipelineDepthAvailable(); \
nsresult SetPipelinePosition(PRInt32); \
PRInt32 PipelinePosition();
//-----------------------------------------------------------------------------
// nsAHttpSegmentReader

View File

@ -57,6 +57,7 @@ HTTP_ATOM(Accept_Ranges, "Accept-Ranges")
HTTP_ATOM(Age, "Age")
HTTP_ATOM(Allow, "Allow")
HTTP_ATOM(Alternate_Protocol, "Alternate-Protocol")
HTTP_ATOM(Assoc_Req, "Assoc-Req")
HTTP_ATOM(Authentication, "Authentication")
HTTP_ATOM(Authorization, "Authorization")
HTTP_ATOM(Cache_Control, "Cache-Control")

View File

@ -47,6 +47,7 @@
#include "nsHttpChannel.h"
#include "nsHttpHandler.h"
#include "nsStandardURL.h"
#include "nsIApplicationCacheService.h"
#include "nsIApplicationCacheContainer.h"
#include "nsIAuthInformation.h"
@ -70,6 +71,7 @@
#include "nsDOMError.h"
#include "nsAlgorithm.h"
#include "sampler.h"
#include "nsIConsoleService.h"
using namespace mozilla;
@ -768,6 +770,10 @@ nsHttpChannel::CallOnStartRequest()
rv = ApplyContentConversions();
if (NS_FAILED(rv)) return rv;
rv = EnsureAssocReq();
if (NS_FAILED(rv))
return rv;
// if this channel is for a download, close off access to the cache.
if (mCacheEntry && mChannelIsForDownload) {
mCacheEntry->Doom();
@ -1712,6 +1718,111 @@ nsHttpChannel::Hash(const char *buf, nsACString &hash)
return NS_OK;
}
nsresult
nsHttpChannel::EnsureAssocReq()
{
// Confirm Assoc-Req response header on pipelined transactions
// per draft-nottingham-http-pipeline-01.txt
// of the form: GET http://blah.com/foo/bar?qv
// return NS_OK as long as we don't find a violation
// (i.e. no header is ok, as are malformed headers, as are
// transactions that have not been pipelined (unless those have been
// opted in via pragma))
if (!mResponseHead)
return NS_OK;
const char *assoc_val = mResponseHead->PeekHeader(nsHttp::Assoc_Req);
if (!assoc_val)
return NS_OK;
if (!mTransaction || !mURI)
return NS_OK;
if (!mTransaction->PipelinePosition()) {
// "Pragma: X-Verify-Assoc-Req" can be used to verify even non pipelined
// transactions. It is used by test harness.
const char *pragma_val = mResponseHead->PeekHeader(nsHttp::Pragma);
if (!pragma_val ||
!nsHttp::FindToken(pragma_val, "X-Verify-Assoc-Req",
HTTP_HEADER_VALUE_SEPS))
return NS_OK;
}
char *method = net_FindCharNotInSet(assoc_val, HTTP_LWS);
if (!method)
return NS_OK;
bool equals;
char *endofmethod;
assoc_val = nsnull;
endofmethod = net_FindCharInSet(method, HTTP_LWS);
if (endofmethod)
assoc_val = net_FindCharNotInSet(endofmethod, HTTP_LWS);
if (!assoc_val)
return NS_OK;
// check the method
PRInt32 methodlen = PL_strlen(mRequestHead.Method().get());
if ((methodlen != (endofmethod - method)) ||
PL_strncmp(method,
mRequestHead.Method().get(),
endofmethod - method)) {
LOG((" Assoc-Req failure Method %s", method));
if (mConnectionInfo)
mConnectionInfo->BanPipelining();
nsCOMPtr<nsIConsoleService> consoleService =
do_GetService(NS_CONSOLESERVICE_CONTRACTID);
if (consoleService) {
nsAutoString message
(NS_LITERAL_STRING("Failed Assoc-Req. Received "));
AppendASCIItoUTF16(
mResponseHead->PeekHeader(nsHttp::Assoc_Req),
message);
message += NS_LITERAL_STRING(" expected method ");
AppendASCIItoUTF16(mRequestHead.Method().get(), message);
consoleService->LogStringMessage(message.get());
}
if (gHttpHandler->EnforceAssocReq())
return NS_ERROR_CORRUPTED_CONTENT;
return NS_OK;
}
// check the URL
nsCOMPtr<nsIURI> assoc_url;
if (NS_FAILED(NS_NewURI(getter_AddRefs(assoc_url), assoc_val)) ||
!assoc_url)
return NS_OK;
mURI->Equals(assoc_url, &equals);
if (!equals) {
LOG((" Assoc-Req failure URL %s", assoc_val));
if (mConnectionInfo)
mConnectionInfo->BanPipelining();
nsCOMPtr<nsIConsoleService> consoleService =
do_GetService(NS_CONSOLESERVICE_CONTRACTID);
if (consoleService) {
nsAutoString message
(NS_LITERAL_STRING("Failed Assoc-Req. Received "));
AppendASCIItoUTF16(
mResponseHead->PeekHeader(nsHttp::Assoc_Req),
message);
message += NS_LITERAL_STRING(" expected URL ");
AppendASCIItoUTF16(mSpec.get(), message);
consoleService->LogStringMessage(message.get());
}
if (gHttpHandler->EnforceAssocReq())
return NS_ERROR_CORRUPTED_CONTENT;
}
return NS_OK;
}
//-----------------------------------------------------------------------------
// nsHttpChannel <byte-range>
//-----------------------------------------------------------------------------

View File

@ -182,6 +182,7 @@ private:
nsresult ContinueProcessFallback(nsresult);
bool ResponseWouldVary();
void HandleAsyncAbort();
nsresult EnsureAssocReq();
nsresult ContinueOnStartRequest1(nsresult);
nsresult ContinueOnStartRequest2(nsresult);

View File

@ -736,7 +736,12 @@ nsHttpConnection::OnHeadersAvailable(nsAHttpTransaction *trans,
}
}
mKeepAliveMask = mKeepAlive;
mConnInfo->SetSupportsPipelining(mSupportsPipelining);
// Update the pipelining status in the connection info object
// and also read it back. It is possible the ci status is
// locked to false if pipelining has been banned on this ci due to
// some kind of observed flaky behavior
mSupportsPipelining = mConnInfo->SetSupportsPipelining(mSupportsPipelining);
// if this connection is persistent, then the server may send a "Keep-Alive"
// header specifying the maximum number of times the connection can be

View File

@ -100,6 +100,27 @@ nsHttpConnectionInfo::Clone() const
return clone;
}
bool
nsHttpConnectionInfo::SupportsPipelining()
{
return mSupportsPipelining;
}
bool
nsHttpConnectionInfo::SetSupportsPipelining(bool support)
{
if (!mBannedPipelining)
mSupportsPipelining = support;
return mSupportsPipelining;
}
void
nsHttpConnectionInfo::BanPipelining()
{
mBannedPipelining = true;
mSupportsPipelining = false;
}
bool
nsHttpConnectionInfo::ShouldForceConnectMethod()
{

View File

@ -61,6 +61,7 @@ public:
, mProxyInfo(proxyInfo)
, mUsingSSL(usingSSL)
, mSupportsPipelining(false)
, mBannedPipelining(false)
{
LOG(("Creating nsHttpConnectionInfo @%x\n", this));
@ -127,13 +128,13 @@ public:
{ mHashKey.SetCharAt(anon ? 'A' : '.', 2); }
bool GetAnonymous() { return mHashKey.CharAt(2) == 'A'; }
bool SupportsPipelining() { return mSupportsPipelining; }
void SetSupportsPipelining(bool support)
{ mSupportsPipelining = support; }
bool ShouldForceConnectMethod();
const nsCString &GetHost() { return mHost; }
bool SupportsPipelining();
bool SetSupportsPipelining(bool support);
void BanPipelining();
private:
nsrefcnt mRef;
nsCString mHashKey;
@ -143,6 +144,7 @@ private:
bool mUsingHttpProxy;
bool mUsingSSL;
bool mSupportsPipelining;
bool mBannedPipelining;
};
#endif // nsHttpConnectionInfo_h__

View File

@ -190,6 +190,7 @@ nsHttpHandler::nsHttpHandler()
, mPhishyUserPassLength(1)
, mQoSBits(0x00)
, mPipeliningOverSSL(false)
, mEnforceAssocReq(false)
, mInPrivateBrowsingMode(PRIVATE_BROWSING_UNKNOWN)
, mLastUniqueID(NowInSeconds())
, mSessionStartTime(0)
@ -1103,6 +1104,13 @@ nsHttpHandler::PrefsChanged(nsIPrefBranch *prefs, const char *pref)
}
}
if (PREF_CHANGED(HTTP_PREF("assoc-req.enforce"))) {
cVar = false;
rv = prefs->GetBoolPref(HTTP_PREF("assoc-req.enforce"), &cVar);
if (NS_SUCCEEDED(rv))
mEnforceAssocReq = cVar;
}
// enable Persistent caching for HTTPS - bug#205921
if (PREF_CHANGED(BROWSER_PREF("disk_cache_ssl"))) {
cVar = false;

View File

@ -110,6 +110,7 @@ public:
PRUint16 GetIdleSynTimeout() { return mIdleSynTimeout; }
bool FastFallbackToIPv4() { return mFastFallbackToIPv4; }
PRUint32 MaxSocketCount();
bool EnforceAssocReq() { return mEnforceAssocReq; }
bool IsPersistentHttpsCachingEnabled() { return mEnablePersistentHttpsCaching; }
bool IsTelemetryEnabled() { return mTelemetryEnabled; }
@ -300,6 +301,7 @@ private:
PRUint8 mQoSBits;
bool mPipeliningOverSSL;
bool mEnforceAssocReq;
// cached value of whether or not the browser is in private browsing mode.
enum {

View File

@ -131,11 +131,20 @@ nsHttpPipeline::AddTransaction(nsAHttpTransaction *trans)
NS_ADDREF(trans);
mRequestQ.AppendElement(trans);
PRInt32 qlen = mRequestQ.Length();
if (qlen != 1) {
trans->SetPipelinePosition(qlen);
}
else {
// do it for this case in case an idempotent cancellation
// is being repeated and an old value needs to be cleared
trans->SetPipelinePosition(0);
}
if (mConnection && !mClosed) {
trans->SetConnection(this);
if (mRequestQ.Length() == 1)
if (qlen == 1)
mConnection->ResumeSend();
}
@ -163,6 +172,24 @@ nsHttpPipeline::PipelineDepthAvailable()
return mMaxPipelineDepth - currentTransactions;
}
nsresult
nsHttpPipeline::SetPipelinePosition(PRInt32 position)
{
nsAHttpTransaction *trans = Response(0);
if (trans)
return trans->SetPipelinePosition(position);
return NS_OK;
}
PRInt32
nsHttpPipeline::PipelinePosition()
{
nsAHttpTransaction *trans = Response(0);
if (trans)
return trans->PipelinePosition();
return 2;
}
//-----------------------------------------------------------------------------
// nsHttpPipeline::nsISupports
//-----------------------------------------------------------------------------

View File

@ -119,6 +119,7 @@ nsHttpTransaction::nsHttpTransaction()
, mPriority(0)
, mRestartCount(0)
, mCaps(0)
, mPipelinePosition(0)
, mClosed(false)
, mConnected(false)
, mHaveStatusLine(false)
@ -722,6 +723,19 @@ nsHttpTransaction::PipelineDepthAvailable()
return 0;
}
nsresult
nsHttpTransaction::SetPipelinePosition(PRInt32 position)
{
mPipelinePosition = position;
return NS_OK;
}
PRInt32
nsHttpTransaction::PipelinePosition()
{
return mPipelinePosition;
}
//-----------------------------------------------------------------------------
// nsHttpTransaction <private>
//-----------------------------------------------------------------------------

View File

@ -199,6 +199,7 @@ private:
PRUint16 mRestartCount; // the number of times this transaction has been restarted
PRUint8 mCaps;
PRInt32 mPipelinePosition;
// state flags, all logically boolean, but not packed together into a
// bitfield so as to avoid bitfield-induced races. See bug 560579.

View File

@ -26,6 +26,7 @@ const CL_EXPECT_GZIP = 0x2;
const CL_EXPECT_3S_DELAY = 0x4;
const CL_SUSPEND = 0x8;
const CL_ALLOW_UNKNOWN_CL = 0x10;
const CL_EXPECT_LATE_FAILURE = 0x20;
const SUSPEND_DELAY = 3000;
@ -38,7 +39,8 @@ const SUSPEND_DELAY = 3000;
*
* This listener makes sure that various parts of the channel API are
* implemented correctly and that the channel's status is a success code
* (you can pass CL_EXPECT_FAILURE as flags to allow a failure code)
* (you can pass CL_EXPECT_FAILURE or CL_EXPECT_LATE_FAILURE as flags
* to allow a failure code)
*
* Note that it also requires a valid content length on the channel and
* is thus not fully generic.
@ -131,15 +133,15 @@ ChannelListener.prototype = {
if (this._got_onstoprequest)
do_throw("Got second onStopRequest event!");
this._got_onstoprequest = true;
if ((this._flags & CL_EXPECT_FAILURE) && success)
if ((this._flags & (CL_EXPECT_FAILURE | CL_EXPECT_LATE_FAILURE)) && success)
do_throw("Should have failed to load URL (status is " + status.toString(16) + ")");
else if (!(this._flags & CL_EXPECT_FAILURE) && !success)
else if (!(this._flags & (CL_EXPECT_FAILURE | CL_EXPECT_LATE_FAILURE)) && !success)
do_throw("Failed to load URL: " + status.toString(16));
if (status != request.status)
do_throw("request.status does not match status arg to onStopRequest!");
if (request.isPending())
do_throw("request reports itself as pending from onStopRequest!");
if (!(this._flags & CL_EXPECT_FAILURE) &&
if (!(this._flags & (CL_EXPECT_FAILURE | CL_EXPECT_LATE_FAILURE)) &&
!(this._flags & CL_EXPECT_GZIP) &&
this._contentLen != -1)
do_check_eq(this._buffer.length, this._contentLen)

View File

@ -0,0 +1,90 @@
do_load_httpd_js();
var httpserver = new nsHttpServer();
var currentTestIndex = 0;
var tests = [
// this is valid
{url: "/assoc/assoctest?valid",
responseheader: [ "Assoc-Req: GET http://localhost:4444/assoc/assoctest?valid",
"Pragma: X-Verify-Assoc-Req" ],
flags : 0},
// this is invalid because the method is wrong
{url: "/assoc/assoctest?invalid",
responseheader: [ "Assoc-Req: POST http://localhost:4444/assoc/assoctest?invalid",
"Pragma: X-Verify-Assoc-Req" ],
flags : CL_EXPECT_LATE_FAILURE},
// this is invalid because the url is wrong
{url: "/assoc/assoctest?notvalid",
responseheader: [ "Assoc-Req: GET http://localhost:4444/wrongpath/assoc/assoctest?notvalid",
"Pragma: X-Verify-Assoc-Req" ],
flags : CL_EXPECT_LATE_FAILURE},
// this is invalid because the space between method and URL is missing
{url: "/assoc/assoctest?invalid2",
responseheader: [ "Assoc-Req: GEThttp://localhost:4444/assoc/assoctest?invalid2",
"Pragma: X-Verify-Assoc-Req" ],
flags : CL_EXPECT_LATE_FAILURE},
];
var oldPrefVal;
var domBranch;
function setupChannel(url)
{
var ios = Components.classes["@mozilla.org/network/io-service;1"].
getService(Ci.nsIIOService);
var chan = ios.newChannel("http://localhost:4444" + url, "", null);
return chan;
}
function startIter()
{
var channel = setupChannel(tests[currentTestIndex].url);
channel.asyncOpen(new ChannelListener(completeIter,
channel, tests[currentTestIndex].flags), null);
}
function completeIter(request, data, ctx)
{
if (++currentTestIndex < tests.length ) {
startIter();
} else {
domBranch.setBoolPref("enforce", oldPrefVal);
httpserver.stop(do_test_finished);
}
}
function run_test()
{
var prefService =
Components.classes["@mozilla.org/preferences-service;1"]
.getService(Components.interfaces.nsIPrefService);
domBranch = prefService.getBranch("network.http.assoc-req.");
oldPrefVal = domBranch.getBoolPref("enforce");
domBranch.setBoolPref("enforce", true);
httpserver.registerPathHandler("/assoc/assoctest", handler);
httpserver.start(4444);
startIter();
do_test_pending();
}
function handler(metadata, response)
{
var body = "thequickbrownfox";
response.setHeader("Content-Type", "text/plain", false);
var header = tests[currentTestIndex].responseheader;
if (header != undefined) {
for (var i = 0; i < header.length; i++) {
var splitHdr = header[i].split(": ");
response.setHeader(splitHdr[0], splitHdr[1], false);
}
}
response.setStatusLine(metadata.httpVersion, 200, "OK");
response.bodyOutputStream.write(body, body.length);
}

View File

@ -6,6 +6,7 @@ tail =
[test_NetUtil.js]
[test_URIs.js]
[test_aboutblank.js]
[test_assoc.js]
[test_auth_proxy.js]
[test_authentication.js]
# Bug 675039: test hangs consistently on Android