Bug 1542172 - Firefox freezes when inspecting large POST data. r=Honza
authorHemakshi Sachdev <sachdev.hemakshi@gmail.com>
Tue, 23 Apr 2019 13:30:42 +0000
changeset 470718 8e100a978832b8844097b54976456a008ff7805c
parent 470717 7d3f2f4c53beb19d3ccd4873a731cf15062170fc
child 470719 305284b53c3c2cf72248c63930f287f4ba909e59
push id112876
push useraciure@mozilla.com
push dateTue, 23 Apr 2019 22:24:17 +0000
treeherdermozilla-inbound@0f9df9c293c3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersHonza
bugs1542172
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1542172 - Firefox freezes when inspecting large POST data. r=Honza Truncate the Request Payload if it exceeds the `devtools.netmonitor.requestBodyLimit` pref and show `Request has been truncated` error in the Params Tab Differential Revision: https://phabricator.services.mozilla.com/D27898
devtools/client/locales/en-US/netmonitor.properties
devtools/client/netmonitor/src/assets/styles/NetworkDetailsPanel.css
devtools/client/netmonitor/src/components/ParamsPanel.js
devtools/client/netmonitor/test/browser.ini
devtools/client/netmonitor/test/browser_net_truncate-post-data.js
devtools/client/netmonitor/test/html_post-json-test-page.html
devtools/server/actors/network-event.js
devtools/server/actors/network-monitor/network-observer.js
devtools/shared/preferences/devtools-shared.js
--- a/devtools/client/locales/en-US/netmonitor.properties
+++ b/devtools/client/locales/en-US/netmonitor.properties
@@ -136,16 +136,21 @@ jsonScopeName=JSON
 # in the response tab of the network details pane for a JSONP scope.
 jsonpScopeName=JSONP → callback %S()
 
 # LOCALIZATION NOTE (responseTruncated): This is the text displayed
 # in the response tab of the network details pane when the response is over
 # the truncation limit and thus was truncated.
 responseTruncated=Response has been truncated
 
+# LOCALIZATION NOTE (requestTruncated): This is the text displayed
+# in the params tab of the network details pane when the request is over
+# the truncation limit and thus was truncated.
+requestTruncated=Request has been truncated
+
 # LOCALIZATION NOTE (responsePreview): This is the text displayed
 # in the response tab of the network details pane for an HTML preview.
 responsePreview=Preview
 
 # LOCALIZATION NOTE (networkMenu.raced): This is the label displayed
 # in the network menu specifying the transfer or a request is
 # raced. %S refers to the current transfer size.
 networkMenu.raced=%S (raced)
--- a/devtools/client/netmonitor/src/assets/styles/NetworkDetailsPanel.css
+++ b/devtools/client/netmonitor/src/assets/styles/NetworkDetailsPanel.css
@@ -322,16 +322,25 @@
   margin-inline-end: 2px;
 }
 
 .network-monitor .headers-summary .status-text {
     width: auto!important;
     margin-left: 4px;
 }
 
+/* Params tabpanel */
+
+.network-monitor .request-error-header {
+  margin: 0;
+  padding: 3px 8px;
+  background-color: var(--theme-highlight-red);
+  color: var(--theme-selection-color);
+}
+
 /* Response tabpanel */
 
 .network-monitor .response-error-header {
   margin: 0;
   padding: 3px 8px;
   background-color: var(--theme-highlight-red);
   color: var(--theme-selection-color);
 }
--- a/devtools/client/netmonitor/src/components/ParamsPanel.js
+++ b/devtools/client/netmonitor/src/components/ParamsPanel.js
@@ -2,16 +2,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 const { Component, createFactory } = require("devtools/client/shared/vendor/react");
 const dom = require("devtools/client/shared/vendor/react-dom-factories");
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
+const Services = require("Services");
 const { connect } = require("devtools/client/shared/redux/visibility-handler-connect");
 const { L10N } = require("../utils/l10n");
 const {
   fetchNetworkUpdatePacket,
   getUrlQuery,
   parseQueryString,
   parseFormData,
 } = require("../utils/request-utils");
@@ -25,16 +26,17 @@ const PropertiesView = createFactory(req
 const { div } = dom;
 
 const JSON_SCOPE_NAME = L10N.getStr("jsonScopeName");
 const PARAMS_EMPTY_TEXT = L10N.getStr("paramsEmptyText");
 const PARAMS_FILTER_TEXT = L10N.getStr("paramsFilterText");
 const PARAMS_FORM_DATA = L10N.getStr("paramsFormData");
 const PARAMS_POST_PAYLOAD = L10N.getStr("paramsPostPayload");
 const PARAMS_QUERY_STRING = L10N.getStr("paramsQueryString");
+const REQUEST_TRUNCATED = L10N.getStr("requestTruncated");
 const SECTION_NAMES = [
   JSON_SCOPE_NAME,
   PARAMS_FORM_DATA,
   PARAMS_POST_PAYLOAD,
   PARAMS_QUERY_STRING,
 ];
 
 /**
@@ -110,52 +112,66 @@ class ParamsPanel extends Component {
 
     if ((!formDataSections || formDataSections.length === 0) && !postData && !query) {
       return div({ className: "empty-notice" },
         PARAMS_EMPTY_TEXT
       );
     }
 
     const object = {};
-    let json;
+    let json, error;
 
     // Query String section
     if (query) {
       object[PARAMS_QUERY_STRING] = this.getProperties(parseQueryString(query));
     }
 
     // Form Data section
     if (formDataSections && formDataSections.length > 0) {
       const sections = formDataSections.filter((str) => /\S/.test(str)).join("&");
       object[PARAMS_FORM_DATA] = this.getProperties(parseFormData(sections));
     }
 
     // Request payload section
+
+    const limit = Services.prefs.getIntPref("devtools.netmonitor.requestBodyLimit");
+    // Check if the request post data has been truncated, in which case no parse should
+    // be attempted.
+    if (postData && limit <= postData.length) {
+      error = REQUEST_TRUNCATED;
+    }
+
     if (formDataSections && formDataSections.length === 0 && postData) {
-      try {
-        json = JSON.parse(postData);
-      } catch (error) {
-        // Continue regardless of parsing error
+      if (!error) {
+        try {
+          json = JSON.parse(postData);
+        } catch (err) {
+          // Continue regardless of parsing error
+        }
+
+        if (json) {
+          object[JSON_SCOPE_NAME] = sortObjectKeys(json);
+        }
       }
 
-      if (json) {
-        object[JSON_SCOPE_NAME] = sortObjectKeys(json);
-      }
       object[PARAMS_POST_PAYLOAD] = {
         EDITOR_CONFIG: {
           text: postData,
           mode: mimeType.replace(/;.+/, ""),
         },
       };
     } else {
       postData = "";
     }
 
     return (
       div({ className: "panel-container" },
+        error && div({ className: "request-error-header", title: error },
+          error
+        ),
         PropertiesView({
           object,
           filterPlaceHolder: PARAMS_FILTER_TEXT,
           sectionNames: SECTION_NAMES,
           openLink,
         })
       )
     );
--- a/devtools/client/netmonitor/test/browser.ini
+++ b/devtools/client/netmonitor/test/browser.ini
@@ -208,11 +208,12 @@ skip-if = true # Bug 1373558
 [browser_net_telemetry_filters_changed.js]
 [browser_net_telemetry_sidepanel_changed.js]
 [browser_net_telemetry_throttle_changed.js]
 [browser_net_throttle.js]
 [browser_net_timeline_ticks.js]
 skip-if = true # TODO: fix the test
 [browser_net_timing-division.js]
 [browser_net_tracking-resources.js]
+[browser_net_truncate-post-data.js]
 [browser_net_truncate.js]
 [browser_net_view-source-debugger.js]
 [browser_net_waterfall-click.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/netmonitor/test/browser_net_truncate-post-data.js
@@ -0,0 +1,54 @@
+/* Any copyright is dedicated to the Public Domain.
+  http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+/**
+  * Bug 1542172 -
+  * Verifies that requests with large post data are truncated and error is displayed.
+*/
+add_task(async function() {
+  const { monitor, tab } = await initNetMonitor(POST_JSON_URL);
+
+  info("Starting test... ");
+
+  const { L10N } = require("devtools/client/netmonitor/src/utils/l10n");
+
+  const { document, store, windowRequire } = monitor.panelWin;
+  const Actions = windowRequire("devtools/client/netmonitor/src/actions/index");
+  store.dispatch(Actions.batchEnable(false));
+
+  requestLongerTimeout(2);
+
+  info("Perform requests");
+  await performRequestsAndWait();
+
+  await waitUntil(() => document.querySelector(".request-list-item"));
+  const item = document.querySelectorAll(".request-list-item")[0];
+  await waitUntil(() => item.querySelector(".requests-list-type").title);
+
+  wait = waitForDOM(document, "#params-panel .tree-section", 1);
+  store.dispatch(Actions.toggleNetworkDetails());
+  EventUtils.sendMouseEvent({ type: "click" }, document.querySelector("#params-tab"));
+  await wait;
+
+  const tabpanel = document.querySelector("#params-panel");
+  is(tabpanel.querySelector(".request-error-header") === null, false,
+    "The request error header doesn't have the intended visibility.");
+  is(tabpanel.querySelector(".request-error-header").textContent,
+    "Request has been truncated", "The error message shown is incorrect");
+  const jsonView = tabpanel.querySelector(".tree-section .treeLabel") || {};
+  is(jsonView.textContent === L10N.getStr("jsonScopeName"), false,
+    "The params json view doesn't have the intended visibility.");
+  is(tabpanel.querySelector("pre") === null, false,
+    "The Request Payload has the intended visibility.");
+
+  return teardown(monitor);
+  async function performRequestsAndWait() {
+    const wait = waitForNetworkEvents(monitor, 1);
+    await ContentTask.spawn(tab.linkedBrowser, {}, async function() {
+      content.wrappedJSObject.performLargePostDataRequest();
+    });
+    await wait;
+  }
+});
--- a/devtools/client/netmonitor/test/html_post-json-test-page.html
+++ b/devtools/client/netmonitor/test/html_post-json-test-page.html
@@ -10,17 +10,17 @@
     <meta http-equiv="Expires" content="0" />
     <title>Network Monitor test page</title>
   </head>
 
   <body>
     <p>POST raw test</p>
 
     <script type="text/javascript">
-      /* exported performRequests */
+      /* exported performRequests performLargePostDataRequest */
       "use strict";
 
       function post(address, message, callback) {
         const xhr = new XMLHttpRequest();
         xhr.open("POST", address, true);
         xhr.setRequestHeader("Content-Type", "application/json");
 
         xhr.onreadystatechange = function() {
@@ -31,12 +31,20 @@
         xhr.send(message);
       }
 
       function performRequests() {
         post("sjs_simple-test-server.sjs", JSON.stringify({a: 1}), function() {
           // Done.
         });
       }
+
+      function performLargePostDataRequest() {
+        const limit = 1048576;
+        const data = "x".repeat(2 * limit);
+        post("sjs_simple-test-server.sjs", JSON.stringify(data), function() {
+          // Done.
+        });
+      }
     </script>
   </body>
 
 </html>
--- a/devtools/server/actors/network-event.js
+++ b/devtools/server/actors/network-event.js
@@ -351,17 +351,17 @@ const NetworkEventActor = protocol.Actor
       return;
     }
 
     this._request.postData = postData;
     postData.text = new LongStringActor(this.conn, postData.text);
     // bug 1462561 - Use "json" type and manually manage/marshall actors to woraround
     // protocol.js performance issue
     this.manage(postData.text);
-    const dataSize = postData.text.str.length;
+    const dataSize = postData.size;
     postData.text = postData.text.form();
 
     this.emit("network-event-update:post-data", "requestPostData", {
       dataSize,
       discardRequestBody: this._discardRequestBody,
     });
   },
 
--- a/devtools/server/actors/network-monitor/network-observer.js
+++ b/devtools/server/actors/network-monitor/network-observer.js
@@ -390,17 +390,25 @@ NetworkObserver.prototype = {
         };
       }
     }
 
     switch (activitySubtype) {
       case gActivityDistributor.ACTIVITY_SUBTYPE_REQUEST_BODY_SENT:
         this._onRequestBodySent(httpActivity);
         if (httpActivity.sentBody !== null) {
-          httpActivity.owner.addRequestPostData({ text: httpActivity.sentBody });
+          const limit = Services.prefs.getIntPref("devtools.netmonitor.requestBodyLimit");
+          const size = httpActivity.sentBody.length;
+          if (size > limit && limit > 0) {
+            httpActivity.sentBody = httpActivity.sentBody.substr(0, limit);
+          }
+          httpActivity.owner.addRequestPostData({
+            text: httpActivity.sentBody,
+            size: size,
+          });
           httpActivity.sentBody = null;
         }
         break;
       case gActivityDistributor.ACTIVITY_SUBTYPE_RESPONSE_HEADER:
         this._onResponseHeader(httpActivity, extraStringData);
         break;
       case gActivityDistributor.ACTIVITY_SUBTYPE_TRANSACTION_CLOSE:
         this._onTransactionClose(httpActivity);
--- a/devtools/shared/preferences/devtools-shared.js
+++ b/devtools/shared/preferences/devtools-shared.js
@@ -33,21 +33,22 @@ pref("devtools.debugger.remote-enabled",
 pref("devtools.debugger.log", false);
 pref("devtools.debugger.log.verbose", false);
 
 pref("devtools.debugger.remote-port", 6000);
 pref("devtools.debugger.remote-websocket", false);
 // Force debugger server binding on the loopback interface
 pref("devtools.debugger.force-local", true);
 
-// Limit for intercepted response bodies (1 MB)
+// Limit for intercepted request and response bodies (1 MB)
 // Possible values:
 // 0 => the response body has no limit
 // n => represents max number of bytes stored
 pref("devtools.netmonitor.responseBodyLimit", 1048576);
+pref("devtools.netmonitor.requestBodyLimit", 1048576);
 
 // DevTools default color unit
 pref("devtools.defaultColorUnit", "authored");
 
 // Used for devtools debugging
 pref("devtools.dump.emit", false);
 
 // Disable device discovery logging