Bug 1384405 - Dump request payload for V2 400 responses. r?francois draft
authorDimiL <dlee@mozilla.com>
Thu, 05 Oct 2017 08:37:29 +0800
changeset 675284 4ee70c3d04d187b6e86fcac49ed4fc9f94cd47d4
parent 675205 dedd9a48da6982467b8b4b635eec59521168ec19
child 734568 30d640f891aa24a9ede7020a15cdb649abf96c2e
push id83093
push userbmo:dlee@mozilla.com
push dateThu, 05 Oct 2017 03:27:21 +0000
reviewersfrancois
bugs1384405
milestone58.0a1
Bug 1384405 - Dump request payload for V2 400 responses. r?francois Refactor SafeBrowsing code so we can get the payload of gethash/update request when updates fail. MozReview-Commit-ID: NB1oacd185
toolkit/components/url-classifier/nsUrlClassifierHashCompleter.js
toolkit/components/url-classifier/nsUrlClassifierStreamUpdater.cpp
toolkit/components/url-classifier/nsUrlClassifierStreamUpdater.h
--- a/toolkit/components/url-classifier/nsUrlClassifierHashCompleter.js
+++ b/toolkit/components/url-classifier/nsUrlClassifierHashCompleter.js
@@ -430,26 +430,30 @@ HashCompleterRequest.prototype = {
     }
   },
 
   // Creates an nsIChannel for the request and fills the body.
   openChannel: function HCR_openChannel() {
     let loadFlags = Ci.nsIChannel.INHIBIT_CACHING |
                     Ci.nsIChannel.LOAD_BYPASS_CACHE;
 
-    this.actualGethashUrl = this.gethashUrl;
+    this.request = {
+      url: this.gethashUrl,
+      body: ""
+    };
+
     if (this.isV4) {
       // As per spec, we add the request payload to the gethash url.
-      this.actualGethashUrl += "&$req=" + this.buildRequestV4();
+      this.request.url += "&$req=" + this.buildRequestV4();
     }
 
-    log("actualGethashUrl: " + this.actualGethashUrl);
+    log("actualGethashUrl: " + this.request.url);
 
     let channel = NetUtil.newChannel({
-      uri: this.actualGethashUrl,
+      uri: this.request.url,
       loadUsingSystemPrincipal: true
     });
     channel.loadFlags = loadFlags;
     channel.loadInfo.originAttributes = {
       // The firstPartyDomain value should sync with NECKO_SAFEBROWSING_FIRST_PARTY_DOMAIN
       // defined in nsNetUtil.h.
       firstPartyDomain: "safebrowsing.86868755-6b82-4842-b301-72671a0db32e.mozilla"
     };
@@ -778,17 +782,17 @@ HashCompleterRequest.prototype = {
     let success = Components.isSuccessCode(aStatusCode);
     log("Received a " + httpStatus + " status code from the " + this.provider +
         " gethash server (success=" + success + ").");
 
     Services.telemetry.getKeyedHistogramById("URLCLASSIFIER_COMPLETE_REMOTE_STATUS2").
       add(this.telemetryProvider, httpStatusToBucket(httpStatus));
     if (httpStatus == 400) {
       dump("Safe Browsing server returned a 400 during completion: request= " +
-           this.actualGethashUrl + "\n");
+           this.request.url + ",payload= " + this.request.body + "\n");
     }
 
     Services.telemetry.getKeyedHistogramById("URLCLASSIFIER_COMPLETE_TIMEOUT2").
       add(this.telemetryProvider, 0);
 
     // Notify the RequestBackoff once a response is received.
     this._completer.finishRequest(this.gethashUrl, httpStatus);
 
--- a/toolkit/components/url-classifier/nsUrlClassifierStreamUpdater.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierStreamUpdater.cpp
@@ -108,19 +108,17 @@ NS_IMPL_ISUPPORTS(nsUrlClassifierStreamU
 void
 nsUrlClassifierStreamUpdater::DownloadDone()
 {
   LOG(("nsUrlClassifierStreamUpdater::DownloadDone [this=%p]", this));
   mIsUpdating = false;
 
   mPendingUpdates.Clear();
   mDownloadError = false;
-  mSuccessCallback = nullptr;
-  mUpdateErrorCallback = nullptr;
-  mDownloadErrorCallback = nullptr;
+  mCurrentRequest = nullptr;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // nsIUrlClassifierStreamUpdater implementation
 
 nsresult
 nsUrlClassifierStreamUpdater::FetchUpdate(nsIURI *aUpdateUrl,
                                           const nsACString & aRequestPayload,
@@ -281,27 +279,23 @@ nsUrlClassifierStreamUpdater::DownloadUp
   NS_ENSURE_ARG(aSuccessCallback);
   NS_ENSURE_ARG(aUpdateErrorCallback);
   NS_ENSURE_ARG(aDownloadErrorCallback);
 
   if (mIsUpdating) {
     LOG(("Already updating, queueing update %s from %s", aRequestPayload.Data(),
          aUpdateUrl.Data()));
     *_retval = false;
-    PendingRequest *request = mPendingRequests.AppendElement(fallible);
+    UpdateRequest *request = mPendingRequests.AppendElement(fallible);
     if (!request) {
       return NS_ERROR_OUT_OF_MEMORY;
     }
-    request->mTables = aRequestTables;
-    request->mRequestPayload = aRequestPayload;
-    request->mIsPostRequest = aIsPostRequest;
-    request->mUrl = aUpdateUrl;
-    request->mSuccessCallback = aSuccessCallback;
-    request->mUpdateErrorCallback = aUpdateErrorCallback;
-    request->mDownloadErrorCallback = aDownloadErrorCallback;
+    BuildUpdateRequest(aRequestTables, aRequestPayload, aIsPostRequest, aUpdateUrl,
+                       aSuccessCallback, aUpdateErrorCallback, aDownloadErrorCallback,
+                       request);
     return NS_OK;
   }
 
   if (aUpdateUrl.IsEmpty()) {
     NS_ERROR("updateUrl not set");
     return NS_ERROR_NOT_INITIALIZED;
   }
 
@@ -324,27 +318,23 @@ nsUrlClassifierStreamUpdater::DownloadUp
     mInitialized = true;
   }
 
   rv = mDBService->BeginUpdate(this, aRequestTables);
   if (rv == NS_ERROR_NOT_AVAILABLE) {
     LOG(("Service busy, already updating, queuing update %s from %s",
          aRequestPayload.Data(), aUpdateUrl.Data()));
     *_retval = false;
-    PendingRequest *request = mPendingRequests.AppendElement(fallible);
+    UpdateRequest *request = mPendingRequests.AppendElement(fallible);
     if (!request) {
       return NS_ERROR_OUT_OF_MEMORY;
     }
-    request->mTables = aRequestTables;
-    request->mRequestPayload = aRequestPayload;
-    request->mIsPostRequest = aIsPostRequest;
-    request->mUrl = aUpdateUrl;
-    request->mSuccessCallback = aSuccessCallback;
-    request->mUpdateErrorCallback = aUpdateErrorCallback;
-    request->mDownloadErrorCallback = aDownloadErrorCallback;
+    BuildUpdateRequest(aRequestTables, aRequestPayload, aIsPostRequest, aUpdateUrl,
+                       aSuccessCallback, aUpdateErrorCallback, aDownloadErrorCallback,
+                       request);
 
     // We cannot guarantee that we will be notified when DBService is done
     // processing the current update, so we fire a retry timer on our own.
     nsresult rv;
     mFetchNextRequestTimer = do_CreateInstance("@mozilla.org/timer;1", &rv);
     if (NS_SUCCEEDED(rv)) {
       rv = mFetchNextRequestTimer->InitWithCallback(this,
                                                     FETCH_NEXT_REQUEST_RETRY_DELAY_MS,
@@ -361,24 +351,25 @@ nsUrlClassifierStreamUpdater::DownloadUp
   nsCOMPtr<nsIUrlClassifierUtils> urlUtil =
     do_GetService(NS_URLCLASSIFIERUTILS_CONTRACTID);
 
   nsTArray<nsCString> tables;
   mozilla::safebrowsing::Classifier::SplitTables(aRequestTables, tables);
   urlUtil->GetTelemetryProvider(tables.SafeElementAt(0, EmptyCString()),
                                 mTelemetryProvider);
 
-  mSuccessCallback = aSuccessCallback;
-  mUpdateErrorCallback = aUpdateErrorCallback;
-  mDownloadErrorCallback = aDownloadErrorCallback;
+  mCurrentRequest = MakeUnique<UpdateRequest>();
+  BuildUpdateRequest(aRequestTables, aRequestPayload, aIsPostRequest, aUpdateUrl,
+                     aSuccessCallback, aUpdateErrorCallback, aDownloadErrorCallback,
+                     mCurrentRequest.get());
 
   mIsUpdating = true;
   *_retval = true;
 
-  LOG(("FetchUpdate: %s", aUpdateUrl.Data()));
+  LOG(("FetchUpdate: %s", mCurrentRequest->mUrl.Data()));
 
   return FetchUpdate(aUpdateUrl, aRequestPayload, aIsPostRequest, EmptyCString());
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // nsIUrlClassifierUpdateObserver implementation
 
 NS_IMETHODIMP
@@ -441,33 +432,55 @@ nsUrlClassifierStreamUpdater::FetchNext(
 nsresult
 nsUrlClassifierStreamUpdater::FetchNextRequest()
 {
   if (mPendingRequests.Length() == 0) {
     LOG(("No more requests, returning"));
     return NS_OK;
   }
 
-  PendingRequest request = mPendingRequests[0];
+  UpdateRequest request = mPendingRequests[0];
   mPendingRequests.RemoveElementAt(0);
   LOG(("Stream updater: fetching next request: %s, %s",
        request.mTables.get(), request.mUrl.get()));
   bool dummy;
   DownloadUpdates(
     request.mTables,
     request.mRequestPayload,
     request.mIsPostRequest,
     request.mUrl,
     request.mSuccessCallback,
     request.mUpdateErrorCallback,
     request.mDownloadErrorCallback,
     &dummy);
   return NS_OK;
 }
 
+void
+nsUrlClassifierStreamUpdater::BuildUpdateRequest(
+  const nsACString &aRequestTables,
+  const nsACString &aRequestPayload,
+  bool aIsPostRequest,
+  const nsACString &aUpdateUrl,
+  nsIUrlClassifierCallback *aSuccessCallback,
+  nsIUrlClassifierCallback *aUpdateErrorCallback,
+  nsIUrlClassifierCallback *aDownloadErrorCallback,
+  UpdateRequest* aRequest)
+{
+  MOZ_ASSERT(aRequest);
+
+  aRequest->mTables = aRequestTables;
+  aRequest->mRequestPayload = aRequestPayload;
+  aRequest->mIsPostRequest = aIsPostRequest;
+  aRequest->mUrl = aUpdateUrl;
+  aRequest->mSuccessCallback = aSuccessCallback;
+  aRequest->mUpdateErrorCallback = aUpdateErrorCallback;
+  aRequest->mDownloadErrorCallback = aDownloadErrorCallback;
+}
+
 NS_IMETHODIMP
 nsUrlClassifierStreamUpdater::StreamFinished(nsresult status,
                                              uint32_t requestedDelay)
 {
   // We are a service and may not be reset with Init between calls, so reset
   // mBeganStream manually.
   mBeganStream = false;
   LOG(("nsUrlClassifierStreamUpdater::StreamFinished [%" PRIx32 ", %d]",
@@ -502,18 +515,21 @@ NS_IMETHODIMP
 nsUrlClassifierStreamUpdater::UpdateSuccess(uint32_t requestedTimeout)
 {
   LOG(("nsUrlClassifierStreamUpdater::UpdateSuccess [this=%p]", this));
   if (mPendingUpdates.Length() != 0) {
     NS_WARNING("Didn't fetch all safebrowsing update redirects");
   }
 
   // DownloadDone() clears mSuccessCallback, so we save it off here.
-  nsCOMPtr<nsIUrlClassifierCallback> successCallback = mDownloadError ? nullptr : mSuccessCallback.get();
-  nsCOMPtr<nsIUrlClassifierCallback> downloadErrorCallback = mDownloadError ? mDownloadErrorCallback.get() : nullptr;
+  nsCOMPtr<nsIUrlClassifierCallback> successCallback =
+    mDownloadError ? nullptr : mCurrentRequest->mSuccessCallback.get();
+  nsCOMPtr<nsIUrlClassifierCallback> downloadErrorCallback =
+    mDownloadError ? mCurrentRequest->mDownloadErrorCallback.get() : nullptr;
+
   DownloadDone();
 
   nsAutoCString strTimeout;
   strTimeout.AppendInt(requestedTimeout);
   if (successCallback) {
     LOG(("nsUrlClassifierStreamUpdater::UpdateSuccess callback [this=%p]",
          this));
     successCallback->HandleEvent(strTimeout);
@@ -530,18 +546,20 @@ nsUrlClassifierStreamUpdater::UpdateSucc
 }
 
 NS_IMETHODIMP
 nsUrlClassifierStreamUpdater::UpdateError(nsresult result)
 {
   LOG(("nsUrlClassifierStreamUpdater::UpdateError [this=%p]", this));
 
   // DownloadDone() clears mUpdateErrorCallback, so we save it off here.
-  nsCOMPtr<nsIUrlClassifierCallback> errorCallback = mDownloadError ? nullptr : mUpdateErrorCallback.get();
-  nsCOMPtr<nsIUrlClassifierCallback> downloadErrorCallback = mDownloadError ? mDownloadErrorCallback.get() : nullptr;
+  nsCOMPtr<nsIUrlClassifierCallback> errorCallback =
+    mDownloadError ? nullptr : mCurrentRequest->mUpdateErrorCallback.get();
+  nsCOMPtr<nsIUrlClassifierCallback> downloadErrorCallback =
+    mDownloadError ? mCurrentRequest->mDownloadErrorCallback.get() : nullptr;
   DownloadDone();
 
   nsAutoCString strResult;
   strResult.AppendInt(static_cast<uint32_t>(result));
   if (errorCallback) {
     errorCallback->HandleEvent(strResult);
   } else if (downloadErrorCallback) {
     LOG(("Notify download error callback in UpdateError [this=%p]", this));
@@ -637,24 +655,19 @@ nsUrlClassifierStreamUpdater::OnStartReq
       NS_ENSURE_SUCCESS(rv, rv);
 
       uint32_t requestStatus;
       rv = httpChannel->GetResponseStatus(&requestStatus);
       NS_ENSURE_SUCCESS(rv, rv);
       mozilla::Telemetry::Accumulate(mozilla::Telemetry::URLCLASSIFIER_UPDATE_REMOTE_STATUS2,
                                      mTelemetryProvider, HTTPStatusToBucket(requestStatus));
       if (requestStatus == 400) {
-        nsCOMPtr<nsIURI> uri;
-        nsAutoCString spec;
-        rv = httpChannel->GetURI(getter_AddRefs(uri));
-        if (NS_SUCCEEDED(rv) && uri) {
-          uri->GetAsciiSpec(spec);
-        }
-        printf_stderr("Safe Browsing server returned a 400 during update: request = %s \n",
-                      spec.get());
+        printf_stderr("Safe Browsing server returned a 400 during update:"
+                       "request url = %s, payload = %s\n",
+                       mCurrentRequest->mUrl.get(), mCurrentRequest->mRequestPayload.get());
       }
 
       LOG(("nsUrlClassifierStreamUpdater::OnStartRequest %s (%d)", succeeded ?
            "succeeded" : "failed", requestStatus));
       if (!succeeded) {
         // 404 or other error, pass error status back
         strStatus.AppendInt(requestStatus);
         downloadError = true;
@@ -663,17 +676,17 @@ nsUrlClassifierStreamUpdater::OnStartReq
   }
 
   if (downloadError) {
     LOG(("nsUrlClassifierStreamUpdater::Download error [this=%p]", this));
     mDownloadError = true;
     mDownloadErrorStatusStr = strStatus;
     status = NS_ERROR_ABORT;
   } else if (NS_SUCCEEDED(status)) {
-    MOZ_ASSERT(mDownloadErrorCallback);
+    MOZ_ASSERT(mCurrentRequest->mDownloadErrorCallback);
     mBeganStream = true;
     LOG(("nsUrlClassifierStreamUpdater::Beginning stream [this=%p]", this));
     rv = mDBService->BeginStream(mStreamTable);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   mStreamTable.Truncate();
 
--- a/toolkit/components/url-classifier/nsUrlClassifierStreamUpdater.h
+++ b/toolkit/components/url-classifier/nsUrlClassifierStreamUpdater.h
@@ -66,16 +66,36 @@ private:
                        bool aIsPostRequest,
                        const nsACString &aTable);
 
   // Fetches the next table, from mPendingUpdates.
   nsresult FetchNext();
   // Fetches the next request, from mPendingRequests
   nsresult FetchNextRequest();
 
+  struct UpdateRequest {
+    nsCString mTables;
+    nsCString mRequestPayload;
+    bool mIsPostRequest;
+    nsCString mUrl;
+    nsCOMPtr<nsIUrlClassifierCallback> mSuccessCallback;
+    nsCOMPtr<nsIUrlClassifierCallback> mUpdateErrorCallback;
+    nsCOMPtr<nsIUrlClassifierCallback> mDownloadErrorCallback;
+  };
+  // Utility function to create an update request.
+  void
+  BuildUpdateRequest(const nsACString &aRequestTables,
+                     const nsACString &aRequestPayload,
+                     bool aIsPostRequest,
+                     const nsACString &aUpdateUrl,
+                     nsIUrlClassifierCallback *aSuccessCallback,
+                     nsIUrlClassifierCallback *aUpdateErrorCallback,
+                     nsIUrlClassifierCallback *aDownloadErrorCallback,
+                     UpdateRequest* aRequest);
+
   bool mIsUpdating;
   bool mInitialized;
   bool mDownloadError;
   bool mBeganStream;
 
   nsCString mDownloadErrorStatusStr;
 
   // Note that mStreamTable is only used by v2, it is empty for v4 update.
@@ -94,36 +114,25 @@ private:
   nsCOMPtr<nsITimer> mFetchNextRequestTimer;
 
   // Timer to abort the download if the server takes too long to respond.
   nsCOMPtr<nsITimer> mResponseTimeoutTimer;
 
   // Timer to abort the download if it takes too long.
   nsCOMPtr<nsITimer> mTimeoutTimer;
 
-  struct PendingRequest {
-    nsCString mTables;
-    nsCString mRequestPayload;
-    bool mIsPostRequest;
-    nsCString mUrl;
-    nsCOMPtr<nsIUrlClassifierCallback> mSuccessCallback;
-    nsCOMPtr<nsIUrlClassifierCallback> mUpdateErrorCallback;
-    nsCOMPtr<nsIUrlClassifierCallback> mDownloadErrorCallback;
-  };
-  nsTArray<PendingRequest> mPendingRequests;
+  mozilla::UniquePtr<UpdateRequest> mCurrentRequest;
+  nsTArray<UpdateRequest> mPendingRequests;
 
   struct PendingUpdate {
     nsCString mUrl;
     nsCString mTable;
   };
   nsTArray<PendingUpdate> mPendingUpdates;
 
-  nsCOMPtr<nsIUrlClassifierCallback> mSuccessCallback;
-  nsCOMPtr<nsIUrlClassifierCallback> mUpdateErrorCallback;
-  nsCOMPtr<nsIUrlClassifierCallback> mDownloadErrorCallback;
 
   // The provider for current update request and should be only used by telemetry
   // since it would show up as "other" for any other providers.
   nsCString mTelemetryProvider;
   PRIntervalTime mTelemetryClockStart;
 };
 
 #endif // nsUrlClassifierStreamUpdater_h_