Skip to content

Commit

Permalink
Additional error checking in Compression API
Browse files Browse the repository at this point in the history
Perform more error checking for DecompressionStream,
which requires adding a new compat flag
  • Loading branch information
fhanau committed Jun 5, 2023
1 parent 1e28618 commit 4460d51
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 25 deletions.
53 changes: 30 additions & 23 deletions src/workerd/api/streams/compression.c++
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,18 @@ public:
DECOMPRESS,
};

enum class ContextFlags {
NONE,
STRICT,
};

struct Result {
bool success = false;
kj::ArrayPtr<const byte> buffer;
};

explicit Context(Mode mode, kj::StringPtr format) : mode(mode) {
explicit Context(Mode mode, kj::StringPtr format, ContextFlags flags) :
mode(mode), strictCompression(flags) {
int result = Z_OK;
switch (mode) {
case Mode::COMPRESS:
Expand Down Expand Up @@ -82,19 +88,16 @@ public:
Error,
"Decompression failed.");

// TODO(soon): The spec requires that a TypeError is produced if there is trailing data
// after the end of the compression stream. This is a potentially breaking change, so just
// put out a warning for now. Later, make it an error and provide a test to confirm that
// input with trailing data causes a TypeError.
JSG_WARN_ONCE_IF(result == Z_STREAM_END && ctx.avail_in > 0,
"Trailing bytes after end of compressed data");

// TODO(soon): Same applies to closing a stream before the complete decompressed data is
// available. Once this is converted to an error, provide a test case checking that
// providing incomplete compressed data results in a TypeError.
JSG_WARN_ONCE_IF(flush == Z_FINISH && result == Z_BUF_ERROR &&
ctx.avail_out == sizeof(buffer),
"Called close() on a decompression stream with incomplete data");
if (strictCompression == ContextFlags::STRICT) {
// The spec requires that a TypeError is produced if there is trailing data after the end
// of the compression stream.
JSG_REQUIRE(!(result == Z_STREAM_END && ctx.avail_in > 0), TypeError,
"Trailing bytes after end of compressed data");
// Same applies to closing a stream before the complete decompressed data is available.
JSG_REQUIRE(!(flush == Z_FINISH && result == Z_BUF_ERROR &&
ctx.avail_out == sizeof(buffer)), TypeError,
"Called close() on a decompression stream with incomplete data");
}
break;
default:
KJ_UNREACHABLE;
Expand Down Expand Up @@ -126,6 +129,9 @@ private:
Mode mode;
z_stream ctx = {};
kj::byte buffer[4096];

// For the eponymous compatibility flag
ContextFlags strictCompression;
};

template <Context::Mode mode>
Expand All @@ -134,8 +140,8 @@ class CompressionStreamImpl: public kj::Refcounted,
public WritableStreamSink {
// Uncompressed data goes in. Compressed data comes out.
public:
explicit CompressionStreamImpl(kj::String format)
: context(mode, format) {}
explicit CompressionStreamImpl(kj::String format, Context::ContextFlags flags)
: context(mode, format, flags) {}

// WritableStreamSink implementation ---------------------------------------------------

Expand Down Expand Up @@ -375,14 +381,13 @@ private:
};
} // namespace

jsg::Ref<CompressionStream> CompressionStream::constructor(
jsg::Lock& js,
kj::String format) {
jsg::Ref<CompressionStream> CompressionStream::constructor(jsg::Lock& js, kj::String format) {
JSG_REQUIRE(format == "deflate" || format == "gzip" || format == "deflate-raw", TypeError,
"The compression format must be either 'deflate', 'deflate-raw' or 'gzip'.");

auto readableSide =
kj::refcounted<CompressionStreamImpl<Context::Mode::COMPRESS>>(kj::mv(format));
kj::refcounted<CompressionStreamImpl<Context::Mode::COMPRESS>>(kj::mv(format),
Context::ContextFlags::NONE);
auto writableSide = kj::addRef(*readableSide);

auto& ioContext = IoContext::current();
Expand All @@ -393,12 +398,14 @@ jsg::Ref<CompressionStream> CompressionStream::constructor(
}

jsg::Ref<DecompressionStream> DecompressionStream::constructor(
jsg::Lock& js,
kj::String format) {
jsg::Lock& js, kj::String format, CompatibilityFlags::Reader flags) {
JSG_REQUIRE(format == "deflate" || format == "gzip" || format == "deflate-raw", TypeError,
"The compression format must be either 'deflate', 'deflate-raw' or 'gzip'.");

auto readableSide =
kj::refcounted<CompressionStreamImpl<Context::Mode::DECOMPRESS>>(kj::mv(format));
kj::refcounted<CompressionStreamImpl<Context::Mode::DECOMPRESS>>(
kj::mv(format), flags.getStrictCompression() ? Context::ContextFlags::STRICT :
Context::ContextFlags::NONE);
auto writableSide = kj::addRef(*readableSide);

auto& ioContext = IoContext::current();
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/api/streams/compression.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class DecompressionStream: public TransformStream {
public:
using TransformStream::TransformStream;

static jsg::Ref<DecompressionStream> constructor(jsg::Lock& js, kj::String format);
static jsg::Ref<DecompressionStream> constructor(jsg::Lock& js, kj::String format, CompatibilityFlags::Reader flags);

JSG_RESOURCE_TYPE(DecompressionStream) {
JSG_INHERIT(TransformStream);
Expand Down
10 changes: 9 additions & 1 deletion src/workerd/io/compatibility-date.capnp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ struct CompatibilityFlags @0x8f8c1b68151b6cef {
# be enabled.

annotation compatEnableAllDates @0x9a1d37c8030d9418 (field) :Void;
# All compatability dates should start using the flag as enabled.
# All compatibility dates should start using the flag as enabled.
# NOTE: This is almost NEVER what you actually want because you're most likely breaking back
# compat. Note that workers uploaded with the flag will fail validation, so this will break
# uploads for anyone still using the flag.
Expand Down Expand Up @@ -304,4 +304,12 @@ struct CompatibilityFlags @0x8f8c1b68151b6cef {
$compatEnableDate("2023-07-01");
# When enabled, the delete() and has() methods of the standard URLSearchParams object
# (see url-standard.h) will have the recently added second value argument enabled.

strictCompression @31 :Bool
$compatEnableFlag("strict_compression_checks")
$compatDisableFlag("no_strict_compression_checks")
$compatEnableDate("2023-08-01");
# Perform additional error checking in the Web Compression API and throw an error if a
# DecompressionStream has trailing data or gets closed before the full compressed data has been
# provided.
}

0 comments on commit 4460d51

Please sign in to comment.