Streams: Tests for abuse of RSBYOBRequest (#9558)

In whatwg/streams#870 it was made impossible to directly construct a ReadableStreamBYOBRequest. Prior to that it was possible to violate the state invariants of ReadableByteStreamController by constructing then using a BYOBRequest directly. Add tests that verify that it is not possible to construct a ReadableStreamBYOBRequest directly. Also add tests that it isn't possible to violate the invariants of ReadableByteStreamController by re-using a BYOBRequest object. Also add a test that it isn't possible to keep a BYOBRequest object valid while releasing the reader lock, as this would also be a way to violate ReadableByteStreamController's invariants. Fixes whatwg/streams#870
web-platform-tests · Feb 19, 2018 · abcd6e9 · abcd6e9
1 parent 0eaf763
commit abcd6e9
Show file tree

Hide file tree

Showing 7 changed files with 215 additions and 0 deletions.
diff --git a/streams/readable-byte-streams/construct-byob-request.dedicatedworker.html b/streams/readable-byte-streams/construct-byob-request.dedicatedworker.html
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>construct-byob-request.js dedicated worker wrapper file</title>
+
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<script>
+'use strict';
+fetch_tests_from_worker(new Worker('construct-byob-request.js'));
+</script>
diff --git a/streams/readable-byte-streams/construct-byob-request.html b/streams/readable-byte-streams/construct-byob-request.html
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>construct-byob-request.js browser context wrapper file</title>
+
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<script src="../resources/rs-utils.js"></script>
+
+<script src="construct-byob-request.js"></script>
diff --git a/streams/readable-byte-streams/construct-byob-request.js b/streams/readable-byte-streams/construct-byob-request.js
@@ -0,0 +1,82 @@
+'use strict';
+
+// Prior to whatwg/stream#870 it was possible to construct a ReadableStreamBYOBRequest directly. This made it possible
+// to construct requests that were out-of-sync with the state of the ReadableStream. They could then be used to call
+// internal operations, resulting in asserts or bad behaviour. This file contains regression tests for the change.
+
+if (self.importScripts) {
+  self.importScripts('../resources/rs-utils.js');
+  self.importScripts('/resources/testharness.js');
+}
+
+function getRealByteStreamController() {
+  let controller;
+  new ReadableStream({
+    start(c) {
+      controller = c;
+    },
+    type: 'bytes'
+  });
+  return controller;
+}
+
+const ReadableByteStreamController = getRealByteStreamController().constructor;
+
+// Create an object pretending to have prototype |prototype|, of type |type|. |type| is one of "undefined", "null",
+// "fake", or "real". "real" will call the realObjectCreator function to get a real instance of the object.
+function createDummyObject(prototype, type, realObjectCreator) {
+  switch (type) {
+    case 'undefined':
+      return undefined;
+
+    case 'null':
+      return null;
+
+    case 'fake':
+      return Object.create(prototype);
+
+    case 'real':
+      return realObjectCreator();
+  }
+
+  throw new Error('not reached');
+}
+
+const dummyTypes = ['undefined', 'null', 'fake', 'real'];
+
+function runTests(ReadableStreamBYOBRequest) {
+  for (const controllerType of dummyTypes) {
+    const controller = createDummyObject(ReadableByteStreamController.prototype, controllerType,
+                                       getRealByteStreamController);
+    for (const viewType of dummyTypes) {
+      const view = createDummyObject(Uint8Array.prototype, viewType, () => new Uint8Array(16));
+      test(() => {
+        assert_throws(new TypeError(), () => new ReadableStreamBYOBRequest(controller, view),
+                      'constructor should throw');
+      }, `ReadableStreamBYOBRequest constructor should throw when passed a ${controllerType} ` +
+         `ReadableByteStreamController and a ${viewType} view`);
+    }
+  }
+}
+
+function getConstructorAndRunTests() {
+  let ReadableStreamBYOBRequest;
+  const rs = new ReadableStream({
+    pull(controller) {
+      const byobRequest = controller.byobRequest;
+      ReadableStreamBYOBRequest = byobRequest.constructor;
+      byobRequest.respond(4);
+    },
+    type: 'bytes'
+  });
+  rs.getReader({ mode: 'byob' }).read(new Uint8Array(8)).then(() => {
+    runTests(ReadableStreamBYOBRequest);
+    done();
+  });
+}
+
+// We can only get at the ReadableStreamBYOBRequest constructor asynchronously, so we need to make the test harness wait
+// for us to explicitly tell it all our tests have run.
+setup({ explicit_done: true });
+
+getConstructorAndRunTests();
diff --git a/streams/readable-byte-streams/construct-byob-request.serviceworker.https.html b/streams/readable-byte-streams/construct-byob-request.serviceworker.https.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>construct-byob-request.js service worker wrapper file</title>
+
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/service-workers/service-worker/resources/test-helpers.sub.js"></script>
+
+<script>
+'use strict';
+service_worker_test('construct-byob-request.js', 'Service worker test setup');
+</script>
diff --git a/streams/readable-byte-streams/construct-byob-request.sharedworker.html b/streams/readable-byte-streams/construct-byob-request.sharedworker.html
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>construct-byob-request.js shared worker wrapper file</title>
+
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<script>
+'use strict';
+fetch_tests_from_worker(new SharedWorker('construct-byob-request.js'));
+</script>
diff --git a/streams/readable-byte-streams/general.html b/streams/readable-byte-streams/general.html
@@ -6,5 +6,6 @@
 <script src="/resources/testharnessreport.js"></script>
 
 <script src="../resources/rs-utils.js"></script>
+<script src="../resources/test-utils.js"></script>
 
 <script src="general.js"></script>
diff --git a/streams/readable-byte-streams/general.js b/streams/readable-byte-streams/general.js
@@ -2,6 +2,7 @@
 
 if (self.importScripts) {
   self.importScripts('../resources/rs-utils.js');
+  self.importScripts('../resources/test-utils.js');
   self.importScripts('/resources/testharness.js');
 }
 
@@ -1882,6 +1883,93 @@ promise_test(t => {
 }, 'ReadableStream with byte source: Throwing in pull in response to read(view) must be ignored if the stream is ' +
    'errored in it');
 
+promise_test(() => {
+  let byobRequest;
+  const rs = new ReadableStream({
+    pull(controller) {
+      byobRequest = controller.byobRequest;
+      byobRequest.respond(4);
+    },
+    type: 'bytes'
+  });
+  const reader = rs.getReader({ mode: 'byob' });
+  const view = new Uint8Array(16);
+  return reader.read(view).then(() => {
+    assert_throws(new TypeError(), () => byobRequest.respond(4), 'respond() should throw a TypeError');
+  });
+}, 'calling respond() twice on the same byobRequest should throw');
+
+promise_test(() => {
+  let byobRequest;
+  const newView = () => new Uint8Array(16);
+  const rs = new ReadableStream({
+    pull(controller) {
+      byobRequest = controller.byobRequest;
+      byobRequest.respondWithNewView(newView());
+    },
+    type: 'bytes'
+  });
+  const reader = rs.getReader({ mode: 'byob' });
+  return reader.read(newView()).then(() => {
+    assert_throws(new TypeError(), () => byobRequest.respondWithNewView(newView()),
+                  'respondWithNewView() should throw a TypeError');
+  });
+}, 'calling respondWithNewView() twice on the same byobRequest should throw');
+
+promise_test(() => {
+  let byobRequest;
+  let resolvePullCalledPromise;
+  const pullCalledPromise = new Promise(resolve => {
+    resolvePullCalledPromise = resolve;
+  });
+  let resolvePull;
+  const rs = new ReadableStream({
+    pull(controller) {
+      byobRequest = controller.byobRequest;
+      resolvePullCalledPromise();
+      return new Promise(resolve => {
+        resolvePull = resolve;
+      });
+    },
+    type: 'bytes'
+  });
+  const reader = rs.getReader({ mode: 'byob' });
+  const readPromise = reader.read(new Uint8Array(16));
+  return pullCalledPromise.then(() => {
+    const cancelPromise = reader.cancel('meh');
+    resolvePull();
+    byobRequest.respond(0);
+    return Promise.all([readPromise, cancelPromise]).then(() => {
+      assert_throws(new TypeError(), () => byobRequest.respond(0), 'respond() should throw');
+    });
+  });
+}, 'calling respond(0) twice on the same byobRequest should throw even when closed');
+
+promise_test(() => {
+  let resolvePullCalledPromise;
+  const pullCalledPromise = new Promise(resolve => {
+    resolvePullCalledPromise = resolve;
+  });
+  let resolvePull;
+  const rs = new ReadableStream({
+    pull() {
+      resolvePullCalledPromise();
+      return new Promise(resolve => {
+        resolvePull = resolve;
+      });
+    },
+    type: 'bytes'
+  });
+  const reader = rs.getReader({ mode: 'byob' });
+  reader.read(new Uint8Array(16));
+  return pullCalledPromise.then(() => {
+    resolvePull();
+    return delay(0).then(() => {
+      assert_throws(new TypeError(), () => reader.releaseLock(), 'releaseLock() should throw');
+    });
+  });
+}, 'pull() resolving should not make releaseLock() possible');
+
 promise_test(() => {
   // Tests https://github.com/whatwg/streams/issues/686