Skip to content

Commit bd2e85b

Browse files
committed
Do not propagate exception inside HTMLRewriter parser
1 parent fd41c48 commit bd2e85b

2 files changed

Lines changed: 66 additions & 2 deletions

File tree

src/workerd/api/html-rewriter.c++

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1263,8 +1263,14 @@ jsg::Ref<Response> HTMLRewriter::transform(jsg::Lock& js, jsg::Ref<Response> res
12631263
// after we know that nothing else (like invalid encoding) could cause an exception.
12641264

12651265
// Drive and flush the parser asynchronously.
1266-
ioContext.addTask(ioContext.waitForDeferredProxy(
1267-
KJ_ASSERT_NONNULL(maybeInput)->pumpTo(js, kj::mv(rewriter), true)));
1266+
ioContext.addTask(
1267+
ioContext
1268+
.waitForDeferredProxy(KJ_ASSERT_NONNULL(maybeInput)->pumpTo(js, kj::mv(rewriter), true))
1269+
.catch_([](kj::Exception&& e) {
1270+
// Errors in pumpTo() are already propagated to the destination stream. We don't want to
1271+
// throw them from here since it'll cause an uncaught exception to be reported via taskFailed(),
1272+
// which would poison the IoContext even though the application may have handled the error.
1273+
}));
12681274

12691275
// TODO(soon): EW-2025 Make Rewriter a proper wrapper object and put it in hidden property on the
12701276
// response so the GC can find the handlers which Rewriter co-owns.

src/workerd/api/tests/htmlrewriter-test.js

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -908,6 +908,64 @@ export const exceptionPropagation = {
908908
},
909909
};
910910

911+
// handled HTMLRewriter errors must not poison the IoContext.
912+
export const continueAfterException = {
913+
async test() {
914+
const errorResponse = new HTMLRewriter()
915+
.on('*', {
916+
element() {
917+
throw new Error('intentional error for testing');
918+
},
919+
})
920+
.transform(new Response('<div>test</div>'));
921+
922+
await rejects(errorResponse.text(), {
923+
message: 'intentional error for testing',
924+
});
925+
926+
const successResponse = new HTMLRewriter()
927+
.on('div', {
928+
element(el) {
929+
el.setInnerContent('success');
930+
},
931+
})
932+
.transform(new Response('<div>original</div>'));
933+
934+
strictEqual(await successResponse.text(), '<div>success</div>');
935+
},
936+
};
937+
938+
export const continueAfterException2 = {
939+
async test() {
940+
const errorResponse = new HTMLRewriter()
941+
.on('*', {
942+
element() {
943+
throw new Error('intentional error for pipeTo testing');
944+
},
945+
})
946+
.transform(new Response('<div>test</div>'));
947+
948+
const { writable } = new TransformStream();
949+
950+
await rejects(errorResponse.body.pipeTo(writable), {
951+
message: 'intentional error for pipeTo testing',
952+
});
953+
954+
const successResponse = new HTMLRewriter()
955+
.on('div', {
956+
element(el) {
957+
el.setInnerContent('success after pipeTo');
958+
},
959+
})
960+
.transform(new Response('<div>original</div>'));
961+
962+
strictEqual(
963+
await successResponse.text(),
964+
'<div>success after pipeTo</div>'
965+
);
966+
},
967+
};
968+
911969
export const sameToken = {
912970
async test() {
913971
const obj = {};

0 commit comments

Comments
 (0)