Skip to content

Commit

Permalink
Bug 1595197 - fix StreamFilter redirect handling r=zombie,robwu
Browse files Browse the repository at this point in the history 
We need to disconnect the stream filter on redirect and allow the extension
to create a new filter.  However, we also must always retain the channel
provided to us in onStartRequest, otherwise onStop fails.  The request passed
to onStartRequest is always the right channel.

Differential Revision: https://phabricator.services.mozilla.com/D53187
  • Loading branch information
@perryjiang
perryjiang committed Apr 29, 2020
1 parent a1af8a6 commit 5846eea
Show file tree
Hide file tree
Showing 3 changed files with 187 additions and 59 deletions.
151 changes: 104 additions & 47 deletions dom/serviceworkers/test/test_streamfilter.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,28 +11,25 @@
</head>
<body>
<script>

add_task(async () => {
add_task(async function setup() {
SimpleTest.waitForExplicitFinish();

info("Setting prefs...")
await SpecialPowers.pushPrefEnv({
set: [
["dom.serviceWorkers.exemptFromPerDomainMax", true],
["dom.serviceWorkers.enabled", true],
["dom.serviceWorkers.testing.enabled", true]
]
["dom.serviceWorkers.exemptFromPerDomainMax", true],
["dom.serviceWorkers.enabled", true],
["dom.serviceWorkers.testing.enabled", true],
],
});

info("Registering Service Worker...");
const registration =
await navigator.serviceWorker.register("streamfilter_worker.js");
const registration = await navigator.serviceWorker.register(
"streamfilter_worker.js"
);

SimpleTest.registerCleanupFunction(async function unregisterRegistration() {
await registration.unregister();
});

info("Waiting to Service Worker to activate and control the current page...");
await new Promise(resolve => {
const serviceWorker = registration.installing;

Expand All @@ -44,53 +41,64 @@
});

ok(navigator.serviceWorker.controller, "Page is controlled");
});

info("Loading WebExtension...");
const webExtension = ExtensionTestUtils.loadExtension({
async function getExtension() {
let extension = ExtensionTestUtils.loadExtension({
manifest: {
permissions: ["webRequest", "webRequestBlocking", "<all_urls>"],
},

// This WebExtension only proxies a response's data through a StreamFilter;
// it doesn't modify the data itself in any way.
background() {
browser.webRequest.onBeforeRequest.addListener(
({ requestId }) => {
class FilterWrapper {
constructor(requestId) {
const filter = browser.webRequest.filterResponseData(requestId);
const arrayBuffers = [];

filter.onstart = () => {
browser.test.sendMessage("start");
};

filter.ondata = ({ data }) => {
arrayBuffers.push(data);
};

filter.onstop = () => {
browser.test.sendMessage("stop");
new Blob(arrayBuffers).arrayBuffer().then(buffer => {
filter.write(buffer);
filter.close();
});
};

filter.onerror = () => {
// We only ever expect a redirect error here.
browser.test.assertEq(filter.error, "Channel redirected");
browser.test.sendMessage("error");
};
}
}

browser.webRequest.onBeforeRequest.addListener(
details => {
new FilterWrapper(details.requestId);
},
{
urls: ["<all_urls>"],
types: ["xmlhttprequest"]
types: ["xmlhttprequest"],
},
["blocking"],
["blocking"]
);
},
});

SimpleTest.registerCleanupFunction(async function unloadWebExtension() {
await webExtension.unload();
});

info("Starting up WebExtension...");
await webExtension.startup();

info("Setup complete!");
});
await extension.startup();
return extension;
}

const streamFilterServerUrl =
`${location.origin}/tests/dom/serviceworkers/test/streamfilter_server.sjs`;
const streamFilterServerUrl = `${location.origin}/tests/dom/serviceworkers/test/streamfilter_server.sjs`;

const requestUrlForServerQueryString = "syntheticResponse=0";

Expand All @@ -102,48 +110,97 @@
// streamfilter_worker.js is expected to respond to a request to this URL.
const requestUrlForServiceWorker = `${streamFilterServerUrl}?${requestUrlForServiceWorkerQueryString}`;

// startNetworkerRequestFn must be a function that, when called, starts a
// network request and returns a promise that resolves after the request
// completes (or fails). This function will return the value that that promise
// resolves with (or throw if it rejects).
async function observeFilteredNetworkRequest(startNetworkRequestFn, promises) {
const networkRequestPromise = startNetworkRequestFn();
await Promise.all(promises);
return networkRequestPromise;
}

// Returns a promise that resolves with the XHR's response text.
function callXHR(requestUrl) {
return new Promise((resolve, reject) => {
const xhr = new XMLHttpRequest();
xhr.onload = () => { resolve(xhr.responseText); };
xhr.onerror = reject;
xhr.open("GET", requestUrl);
xhr.send();
});
function callXHR(requestUrl, promises) {
return observeFilteredNetworkRequest(() => {
return new Promise((resolve, reject) => {
const xhr = new XMLHttpRequest();
xhr.onload = () => {
resolve(xhr.responseText);
};
xhr.onerror = reject;
xhr.open("GET", requestUrl);
xhr.send();
});
}, promises);
}

// Returns a promise that resolves with the Fetch's response text.
function callFetch(requestUrl) {
return fetch(requestUrl).then(response => response.text());
function callFetch(requestUrl, promises) {
return observeFilteredNetworkRequest(() => {
return fetch(requestUrl).then(response => response.text());
}, promises);
}

// The expected response text is always the query string (without the leading
// "?") of the request URL.
add_task(async function callXhrExpectServerResponse() {
info(`Performing XHR at ${requestUrlForServer}...`);
is(await callXHR(requestUrlForServer), requestUrlForServerQueryString,
"Server-supplied response for XHR completed successfully");
let extension = await getExtension();
is(
await callXHR(requestUrlForServer, [
extension.awaitMessage("start"),
extension.awaitMessage("error"),
extension.awaitMessage("stop"),
]),
requestUrlForServerQueryString,
"Server-supplied response for XHR completed successfully"
);
await extension.unload();
});

add_task(async function callXhrExpectServiceWorkerResponse() {
info(`Performing XHR at ${requestUrlForServiceWorker}...`);
is(await callXHR(requestUrlForServiceWorker), requestUrlForServiceWorkerQueryString,
"ServiceWorker-supplied response for XHR completed successfully");
let extension = await getExtension();
is(
await callXHR(requestUrlForServiceWorker, [
extension.awaitMessage("start"),
extension.awaitMessage("stop"),
]),
requestUrlForServiceWorkerQueryString,
"ServiceWorker-supplied response for XHR completed successfully"
);
await extension.unload();
});

add_task(async function callFetchExpectServerResponse() {
info(`Performing Fetch at ${requestUrlForServer}...`);
is(await callFetch(requestUrlForServer), requestUrlForServerQueryString,
"Server-supplied response for Fetch completed successfully");
let extension = await getExtension();
is(
await callFetch(requestUrlForServer, [
extension.awaitMessage("start"),
extension.awaitMessage("error"),
extension.awaitMessage("stop"),
]),
requestUrlForServerQueryString,
"Server-supplied response for Fetch completed successfully"
);
await extension.unload();
});

add_task(async function callFetchExpectServiceWorkerResponse() {
info(`Performing Fetch at ${requestUrlForServiceWorker}...`);
is(await callFetch(requestUrlForServiceWorker), requestUrlForServiceWorkerQueryString,
"ServiceWorker-supplied response for Fetch completed successfully");
let extension = await getExtension();
is(
await callFetch(requestUrlForServiceWorker, [
extension.awaitMessage("start"),
extension.awaitMessage("stop"),
]),
requestUrlForServiceWorkerQueryString,
"ServiceWorker-supplied response for Fetch completed successfully"
);
await extension.unload();
});

</script>
</body>
</html>
73 changes: 73 additions & 0 deletions toolkit/components/extensions/test/xpcshell/test_ext_webRequest_filterResponseData.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,25 @@ server.registerPathHandler("/redirect301", (request, response) => {
response.setHeader("Access-Control-Allow-Origin", "*");
});

server.registerPathHandler("/script302.js", (request, response) => {
response.setStatusLine(request.httpVersion, 302, "Moved Temporarily");
response.setHeader("Location", "http://example.com/script.js");
});

server.registerPathHandler("/script.js", (request, response) => {
response.setStatusLine(request.httpVersion, 200, "OK");
response.setHeader("Content-Type", "application/javascript");
response.write(String.raw`console.log("HELLO!");`);
});

server.registerPathHandler("/302.html", (request, response) => {
response.setStatusLine(request.httpVersion, 200, "OK");
response.setHeader("Content-Type", "text/html");
response.write(String.raw`
<script type="application/javascript" src="http://example.com/script302.js"></script>
`);
});

server.registerPathHandler("/dummy", (request, response) => {
response.setStatusLine(request.httpVersion, 200, "OK");
response.setHeader("Access-Control-Allow-Origin", "*");
Expand Down Expand Up @@ -346,6 +365,60 @@ add_task(async function test_filter_301() {
await extension.unload();
});

add_task(async function test_filter_302() {
let extension = ExtensionTestUtils.loadExtension({
background() {
browser.webRequest.onBeforeRequest.addListener(
details => {
let filter = browser.webRequest.filterResponseData(details.requestId);
browser.test.sendMessage("filter-created");

filter.ondata = event => {
const script = "forceError();";
filter.write(
new Uint8Array(new TextEncoder("utf-8").encode(script))
);
filter.close();
browser.test.sendMessage("filter-ondata");
};

filter.onerror = () => {
browser.test.assertEq(filter.error, "Channel redirected");
browser.test.sendMessage("filter-redirect");
};
},
{
urls: ["http://example.com/*.js"],
},
["blocking"]
);
},

manifest: {
permissions: ["webRequest", "webRequestBlocking", "http://example.com/"],
},
});

await extension.startup();

let { messages } = await promiseConsoleOutput(async () => {
let contentPage = await ExtensionTestUtils.loadContentPage(
"http://example.com/302.html"
);

await extension.awaitMessage("filter-created");
await extension.awaitMessage("filter-redirect");
await extension.awaitMessage("filter-created");
await extension.awaitMessage("filter-ondata");
await contentPage.close();
});
AddonTestUtils.checkMessages(messages, {
expected: [{ message: /forceError is not defined/ }],
});

await extension.unload();
});

add_task(async function test_alternate_cached_data() {
Services.prefs.setBoolPref("dom.script_loader.bytecode_cache.enabled", true);
Services.prefs.setIntPref("dom.script_loader.bytecode_cache.strategy", -1);
Expand Down
22 changes: 10 additions & 12 deletions toolkit/components/extensions/webrequest/StreamFilterParent.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -462,22 +462,19 @@ NS_IMETHODIMP
StreamFilterParent::OnStartRequest(nsIRequest* aRequest) {
AssertIsMainThread();

// If a StreamFilter's request results in an external redirect, that
// StreamFilter should not monitor the redirected request's response (although
// an identical StreamFilter may be created for it). A StreamFilter should,
// however, monitor the response of an "internal" redirect (i.e. a
// browser-initiated rather than server-initiated redirect); in this case,
// mChannel must be replaced.
// Always reset mChannel if aRequest is different. Various calls in
// StreamFilterParent will use mChannel, but aRequest is *always* the
// right channel to use at this point.
//
// For ALL redirections, we will disconnect this listener. Extensions
// will create a new filter if they need it.
if (aRequest != mChannel) {
nsCOMPtr<nsIChannel> channel = do_QueryInterface(aRequest);
nsCOMPtr<nsILoadInfo> loadInfo = channel ? channel->LoadInfo() : nullptr;
mChannel = channel;

if (loadInfo && loadInfo->RedirectChain().IsEmpty()) {
MOZ_DIAGNOSTIC_ASSERT(
!loadInfo->RedirectChainIncludingInternalRedirects().IsEmpty(),
"We should be performing an internal redirect.");
mChannel = channel;
} else {
if (!(loadInfo &&
loadInfo->RedirectChainIncludingInternalRedirects().IsEmpty())) {
mDisconnected = true;

RefPtr<StreamFilterParent> self(this);
Expand Down Expand Up @@ -549,6 +546,7 @@ StreamFilterParent::OnStartRequest(nsIRequest* aRequest) {
NS_IMETHODIMP
StreamFilterParent::OnStopRequest(nsIRequest* aRequest, nsresult aStatusCode) {
AssertIsMainThread();
MOZ_ASSERT(aRequest == mChannel);

mReceivedStop = true;
if (mDisconnected) {
Expand Down

0 comments on commit 5846eea

Please sign in to comment.