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 21, 2023
1 parent 55880d1 commit e7f22da
Showing 1 changed file with 12 additions and 5 deletions.
17 changes: 12 additions & 5 deletions c++/src/kj/compat/http.c++
Expand Up @@ -2574,6 +2574,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 @@ -2584,6 +2585,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 @@ -2632,7 +2634,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 @@ -2651,16 +2654,18 @@ public:
KJ_UNREACHABLE;
case OPCODE_TEXT:
#if KJ_HAS_ZLIB
KJ_IF_MAYBE(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.
const byte tailBytes[] = {0x00, 0x00, 0xFF, 0xFF};
memcpy(tail.begin(), tailBytes, sizeof(tailBytes));
// We have to append 0x00 0x00 0xFF 0xFF to the message before inflating.
// See: https://datatracker.ietf.org/doc/html/rfc7692#section-7.2.2
if (config->inboundNoContextTakeover) {
if (config.inboundNoContextTakeover) {
// We must reset context on each message.
decompressor.reset();
}
Expand All @@ -2675,16 +2680,18 @@ public:
return Message(kj::String(message.releaseAsChars()));
case OPCODE_BINARY:
#if KJ_HAS_ZLIB
KJ_IF_MAYBE(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.
const byte tailBytes[] = {0x00, 0x00, 0xFF, 0xFF};
memcpy(tail.begin(), tailBytes, sizeof(tailBytes));
// We have to append 0x00 0x00 0xFF 0xFF to the message before inflating.
// See: https://datatracker.ietf.org/doc/html/rfc7692#section-7.2.2
if (config->inboundNoContextTakeover) {
if (config.inboundNoContextTakeover) {
// We must reset context on each message.
decompressor.reset();
}
Expand Down

0 comments on commit e7f22da

Please sign in to comment.