Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better error for opaque stream sources #3001

Merged
merged 1 commit into from Dec 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
46 changes: 46 additions & 0 deletions infra/testing/server/cross-origin-server.js
@@ -0,0 +1,46 @@
/*
Copyright 2021 Google LLC

Use of this source code is governed by an MIT-style
license that can be found in the LICENSE file or at
https://opensource.org/licenses/MIT.
*/

const express = require('express');

const PORT = 3010;

let app;
let server;

function initApp(staticDir) {
app = express();
app.use(express.static(staticDir));
}

function start(staticDir) {
if (!app) {
initApp(staticDir);
}

return new Promise((resolve, reject) => {
server = app.listen(PORT, (error) => {
if (error) {
reject(error);
} else {
resolve(`http://localhost:${PORT}`);
}
});
});
}

function stop() {
if (server) {
server.close();
}
}

module.exports = {
start,
stop,
};
13 changes: 13 additions & 0 deletions packages/workbox-core/src/models/messages/messages.ts
Expand Up @@ -366,4 +366,17 @@ export const messages: MessageMap = {
`responses. It was passed a response with origin ${origin}.`
);
},

'opaque-streams-source': ({type}) => {
const message =
`One of the workbox-streams sources resulted in an ` +
`'${type}' response.`;
if (type === 'opaqueredirect') {
return (
`${message} Please do not use a navigation request that results ` +
`in a redirect as a source.`
);
}
return `${message} Please ensure your sources are CORS-enabled.`;
},
};
10 changes: 8 additions & 2 deletions packages/workbox-streams/src/concatenate.ts
Expand Up @@ -6,10 +6,12 @@
https://opensource.org/licenses/MIT.
*/

import {logger} from 'workbox-core/_private/logger.js';
import {assert} from 'workbox-core/_private/assert.js';
import {Deferred} from 'workbox-core/_private/Deferred.js';
import {logger} from 'workbox-core/_private/logger.js';
import {StreamSource} from './_types.js';
import {WorkboxError} from 'workbox-core/_private/WorkboxError.js';

import './_version.js';

/**
Expand All @@ -25,7 +27,11 @@ function _getReaderFromSource(
source: StreamSource,
): ReadableStreamReader<unknown> {
if (source instanceof Response) {
return source.body!.getReader();
// See https://github.com/GoogleChrome/workbox/issues/2998
if (source.body) {
return source.body.getReader();
}
throw new WorkboxError('opaque-streams-source', {type: source.type});
}
if (source instanceof ReadableStream) {
return source.getReader();
Expand Down
38 changes: 37 additions & 1 deletion test/workbox-streams/integration/test-all.js
Expand Up @@ -7,8 +7,10 @@
*/

const {expect} = require('chai');
const activateAndControlSW = require('../../../infra/testing/activate-and-control');
const {runUnitTests} = require('../../../infra/testing/webdriver/runUnitTests');
const activateAndControlSW = require('../../../infra/testing/activate-and-control');
const crossOriginServer = require('../../../infra/testing/server/cross-origin-server');
const upath = require('upath');

// Store local references of these globals.
const {webdriver, server} = global.__workbox;
Expand All @@ -23,10 +25,19 @@ describe(`[workbox-streams] Integration Tests`, function () {
const testServerAddress = server.getAddress();
const testingURL = `${testServerAddress}/test/workbox-streams/static/`;
const swURL = `${testingURL}sw.js`;
let crossOriginURL;

before(async function () {
await webdriver.get(testingURL);
await activateAndControlSW(swURL);
const crossOrigin = await crossOriginServer.start(
upath.join('..', 'static'),
);
crossOriginURL = `${crossOrigin}/4.txt`;
});

after(function () {
crossOriginServer.stop();
});

for (const testCase of ['concatenate', 'concatenateToResponse', 'strategy']) {
Expand Down Expand Up @@ -58,4 +69,29 @@ describe(`[workbox-streams] Integration Tests`, function () {
}
});
}

it(`should error when a stream source results in an opaque response`, async function () {
const {text} = await webdriver.executeAsyncScript(
async (crossOriginURL, cb) => {
try {
const url = new URL('/crossOriginURL', location);
url.searchParams.set('cross-origin-url', crossOriginURL);

const response = await fetch(url);
const text = await response.text();
cb({text});
} catch (error) {
cb({text: error.name});
}
},
crossOriginURL,
);

if (text === 'No streams support') {
this.skip();
} else {
// The exception name varies from browser to browser.
expect(text).to.be.oneOf(['TypeError', 'AbortError']);
}
});
});
23 changes: 20 additions & 3 deletions test/workbox-streams/static/sw.js
Expand Up @@ -23,9 +23,26 @@ const getSourceFunctions = () => [
];

self.addEventListener('fetch', (event) => {
const url = new URL(event.request.url);
const crossOriginURL = url.searchParams.get('cross-origin-url');

if (!workbox.streams.isSupported()) {
event.respondWith(new Response('No streams support'));
} else if (event.request.url.endsWith('concatenateToResponse')) {
} else if (crossOriginURL) {
const {done, response} = workbox.streams.concatenateToResponse(
[
() => 'this will',
() => fetch(crossOriginURL, {mode: 'no-cors'}),
() => 'error',
].map((f) => f()),
{
'content-type': 'text/plain',
'x-test-case': 'crossOriginURL',
},
);
event.respondWith(response);
event.waitUntil(done);
} else if (url.pathname.endsWith('concatenateToResponse')) {
const {done, response} = workbox.streams.concatenateToResponse(
getSourceFunctions().map((f) => f()),
{
Expand All @@ -35,7 +52,7 @@ self.addEventListener('fetch', (event) => {
);
event.respondWith(response);
event.waitUntil(done);
} else if (event.request.url.endsWith('concatenate')) {
} else if (url.pathname.endsWith('concatenate')) {
const {done, stream} = workbox.streams.concatenate(
getSourceFunctions().map((f) => f()),
);
Expand All @@ -51,7 +68,7 @@ self.addEventListener('fetch', (event) => {
});

workbox.routing.registerRoute(
new RegExp('strategy$'),
({url}) => url.pathname.endsWith('strategy'),
workbox.streams.strategy(getSourceFunctions(), {
'content-type': 'text/plain',
'x-test-case': 'strategy',
Expand Down