Skip to content

Commit

Permalink
Fix inconsistent decision about whether a WebSocket message is compre…
Browse files Browse the repository at this point in the history
…ssed.

When processing the header, we decide whether the mesasge is compressed based on the presence of the appropriate flag in the header. However, after receiving the whole message, we were expecting it to be compressed based only on whether the session had negotiated compression upfront. With this patch we only rely on the bits in the header (and throw if the header says it is compressed, but we didn't negotiate compression.)
  • Loading branch information
kentonv committed Nov 16, 2023
1 parent cd841b4 commit 75c5c14
Showing 1 changed file with 10 additions and 3 deletions.
13 changes: 10 additions & 3 deletions c++/src/kj/compat/http.c++
Expand Up @@ -2761,6 +2761,7 @@ public:
}

bool isFin = recvHeader.isFin();
bool isCompressed = false;

kj::Array<byte> message; // space to allocate
byte* payloadTarget; // location into which to read payload (size is payloadLen)
Expand All @@ -2771,6 +2772,7 @@ public:
// Add 4 since we append 0x00 0x00 0xFF 0xFF to the tail of the payload.
// See: https://datatracker.ietf.org/doc/html/rfc7692#section-7.2.2
amountToAllocate = payloadLen + 4;
isCompressed = true;
} else {
// Add space for NUL terminator when allocating text message.
amountToAllocate = payloadLen + (opcode == OPCODE_TEXT && isFin);
Expand Down Expand Up @@ -2819,7 +2821,8 @@ public:
Mask mask = recvHeader.getMask();

auto handleMessage =
[this,opcode,payloadTarget,payloadLen,mask,isFin,maxSize,originalMaxSize,message=kj::mv(message)]() mutable
[this,opcode,payloadTarget,payloadLen,mask,isFin,maxSize,originalMaxSize,
isCompressed,message=kj::mv(message)]() mutable
-> kj::Promise<Message> {
if (!mask.isZero()) {
mask.apply(kj::arrayPtr(payloadTarget, payloadLen));
Expand All @@ -2838,8 +2841,10 @@ public:
KJ_UNREACHABLE;
case OPCODE_TEXT:
#if KJ_HAS_ZLIB
KJ_IF_SOME(config, compressionConfig) {
if (isCompressed) {
auto& config = KJ_ASSERT_NONNULL(compressionConfig);
auto& decompressor = KJ_ASSERT_NONNULL(decompressionContext);
KJ_ASSERT(message.size() >= 4);
auto tail = message.slice(message.size() - 4, message.size());
// Note that we added an additional 4 bytes to `message`s capacity to account for these
// extra bytes. See `amountToAllocate` in the if(recvHeader.isCompressed()) block above.
Expand All @@ -2862,8 +2867,10 @@ public:
return Message(kj::String(message.releaseAsChars()));
case OPCODE_BINARY:
#if KJ_HAS_ZLIB
KJ_IF_SOME(config, compressionConfig) {
if (isCompressed) {
auto& config = KJ_ASSERT_NONNULL(compressionConfig);
auto& decompressor = KJ_ASSERT_NONNULL(decompressionContext);
KJ_ASSERT(message.size() >= 4);
auto tail = message.slice(message.size() - 4, message.size());
// Note that we added an additional 4 bytes to `message`s capacity to account for these
// extra bytes. See `amountToAllocate` in the if(recvHeader.isCompressed()) block above.
Expand Down

0 comments on commit 75c5c14

Please sign in to comment.