grpc-js: fix Buffer-only .copy() crash on Uint8Array messages in CompressionFilter#3066
Open
chatman-media wants to merge 1 commit into
Open
grpc-js: fix Buffer-only .copy() crash on Uint8Array messages in CompressionFilter#3066chatman-media wants to merge 1 commit into
chatman-media wants to merge 1 commit into
Conversation
… compression filter CompressionFilter's writeMessage methods call message.copy(output, 5) to frame outgoing messages. copy() only exists on Buffer, so if a request serializer hands back a plain Uint8Array instead (protobufjs does this when its Buffer util is unavailable, e.g. under some bundlers), this throws 'TypeError: message.copy is not a function' deep inside grpc-js with no useful context. Buffer.prototype.set works for both types, so swapping to that keeps the framing logic identical while fixing the crash. Added a test that exercises both the identity and non-identity (NoCompress) code paths with a Uint8Array payload.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CompressionFilter's writeMessage (both the shared base implementation and IdentityHandler's override) calls
message.copy(output, 5)to frame the outgoing message..copy()only exists onBuffer, but the message here comes straight from the caller'sserializefunction, which is only nominally typed as returning aBuffer— nothing enforces that at runtime. protobufjs in particular falls back to a plainUint8ArrayfromWriter.finish()when its optional Buffer util isn't available (some bundlers strip it out), so this can throwTypeError: message.copy is not a functiondeep inside grpc-js with basically no context for whoever hits it.Buffer.prototype.set(inherited fromUint8Array) does the same job and works for both types, so this just swaps.copy()for.set()in both spots. Added a test that sends aUint8Arraypayload through both the identity path and a non-identity handler with theNoCompressflag, since both call sites had the same bug.Ref #3050.