From e8ed10a4575dcfd6f63275bb423df34c169f62f7 Mon Sep 17 00:00:00 2001 From: Hemakshi Sachdev Date: Tue, 23 Apr 2019 13:30:42 +0000 Subject: [PATCH] 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 --HG-- extra : moz-landing-system : lando --- .../locales/en-US/netmonitor.properties | 5 ++ .../src/assets/styles/NetworkDetailsPanel.css | 9 ++++ .../netmonitor/src/components/ParamsPanel.js | 32 ++++++++--- devtools/client/netmonitor/test/browser.ini | 1 + .../test/browser_net_truncate-post-data.js | 54 +++++++++++++++++++ .../test/html_post-json-test-page.html | 10 +++- devtools/server/actors/network-event.js | 2 +- .../network-monitor/network-observer.js | 10 +++- .../shared/preferences/devtools-shared.js | 3 +- 9 files changed, 114 insertions(+), 12 deletions(-) create mode 100644 devtools/client/netmonitor/test/browser_net_truncate-post-data.js diff --git a/devtools/client/locales/en-US/netmonitor.properties b/devtools/client/locales/en-US/netmonitor.properties index d27e2faa7025..6e6ffc23b2b2 100644 --- a/devtools/client/locales/en-US/netmonitor.properties +++ b/devtools/client/locales/en-US/netmonitor.properties @@ -141,6 +141,11 @@ jsonpScopeName=JSONP → callback %S() # 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 diff --git a/devtools/client/netmonitor/src/assets/styles/NetworkDetailsPanel.css b/devtools/client/netmonitor/src/assets/styles/NetworkDetailsPanel.css index e6d7445e5508..c4e0597d32cd 100644 --- a/devtools/client/netmonitor/src/assets/styles/NetworkDetailsPanel.css +++ b/devtools/client/netmonitor/src/assets/styles/NetworkDetailsPanel.css @@ -327,6 +327,15 @@ 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 { diff --git a/devtools/client/netmonitor/src/components/ParamsPanel.js b/devtools/client/netmonitor/src/components/ParamsPanel.js index ace5d6307a9c..3ba2d054a75a 100644 --- a/devtools/client/netmonitor/src/components/ParamsPanel.js +++ b/devtools/client/netmonitor/src/components/ParamsPanel.js @@ -7,6 +7,7 @@ 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 { @@ -30,6 +31,7 @@ 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, @@ -115,7 +117,7 @@ class ParamsPanel extends Component { } const object = {}; - let json; + let json, error; // Query String section if (query) { @@ -129,16 +131,27 @@ class ParamsPanel extends Component { } // 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, @@ -151,6 +164,9 @@ class ParamsPanel extends Component { return ( div({ className: "panel-container" }, + error && div({ className: "request-error-header", title: error }, + error + ), PropertiesView({ object, filterPlaceHolder: PARAMS_FILTER_TEXT, diff --git a/devtools/client/netmonitor/test/browser.ini b/devtools/client/netmonitor/test/browser.ini index 5d788521ff4f..609d93ee3ea7 100644 --- a/devtools/client/netmonitor/test/browser.ini +++ b/devtools/client/netmonitor/test/browser.ini @@ -213,6 +213,7 @@ skip-if = true # Bug 1373558 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] diff --git a/devtools/client/netmonitor/test/browser_net_truncate-post-data.js b/devtools/client/netmonitor/test/browser_net_truncate-post-data.js new file mode 100644 index 000000000000..841ac6b27a6f --- /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; + } +}); diff --git a/devtools/client/netmonitor/test/html_post-json-test-page.html b/devtools/client/netmonitor/test/html_post-json-test-page.html index ac42c6bbfc87..05a1fbc49b64 100644 --- a/devtools/client/netmonitor/test/html_post-json-test-page.html +++ b/devtools/client/netmonitor/test/html_post-json-test-page.html @@ -15,7 +15,7 @@

POST raw test

diff --git a/devtools/server/actors/network-event.js b/devtools/server/actors/network-event.js index 39ca42c8d4af..cb722ca1f712 100644 --- a/devtools/server/actors/network-event.js +++ b/devtools/server/actors/network-event.js @@ -356,7 +356,7 @@ const NetworkEventActor = protocol.ActorClassWithSpec(networkEventSpec, { // 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", { diff --git a/devtools/server/actors/network-monitor/network-observer.js b/devtools/server/actors/network-monitor/network-observer.js index ee2492fd311b..fb30b8d88932 100644 --- a/devtools/server/actors/network-monitor/network-observer.js +++ b/devtools/server/actors/network-monitor/network-observer.js @@ -395,7 +395,15 @@ NetworkObserver.prototype = { 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; diff --git a/devtools/shared/preferences/devtools-shared.js b/devtools/shared/preferences/devtools-shared.js index 7c47f98fa223..bceb76a7c035 100644 --- a/devtools/shared/preferences/devtools-shared.js +++ b/devtools/shared/preferences/devtools-shared.js @@ -38,11 +38,12 @@ 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");