Skip to content

fix(@libp2p/webrtc): set max message size in alignment with spec #2050

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Sep 15, 2023
Merged

fix(@libp2p/webrtc): set max message size in alignment with spec #2050

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
851635f
fix(@libp2p/webrtc): set max message size in alignment with spec (#2003)
maschad Sep 13, 2023
ce1197b
Merge branch 'master' into fix/update-webrtc-max-message-length
maschad Sep 14, 2023
b306707
test: update test for max message size
maschad Sep 14, 2023
18a0521
refactor: export constants from stream
maschad Sep 15, 2023
c19b8e4
Merge branch 'master' into fix/update-webrtc-max-message-length
maschad Sep 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions packages/transport-webrtc/src/stream.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ const MAX_BUFFERED_AMOUNT = 16 * 1024 * 1024
const BUFFERED_AMOUNT_LOW_TIMEOUT = 30 * 1000

// protobuf field definition overhead
const PROTOBUF_OVERHEAD = 3
const PROTOBUF_OVERHEAD = 5

// Length of varint, in bytes.
const VARINT_LENGTH = 2

export class WebRTCStream extends AbstractStream {
/**
Expand All @@ -58,6 +61,10 @@ export class WebRTCStream extends AbstractStream {
private readonly incomingData: Pushable<Uint8Array>

private messageQueue?: Uint8ArrayList

/**
* The maximum size of a message in bytes
*/
private readonly maxDataSize: number

constructor (init: WebRTCStreamInit) {
Expand All @@ -70,7 +77,7 @@ export class WebRTCStream extends AbstractStream {
this.dataChannelOptions = {
bufferedAmountLowEventTimeout: init.dataChannelOptions?.bufferedAmountLowEventTimeout ?? BUFFERED_AMOUNT_LOW_TIMEOUT,
maxBufferedAmount: init.dataChannelOptions?.maxBufferedAmount ?? MAX_BUFFERED_AMOUNT,
maxMessageSize: init.dataChannelOptions?.maxMessageSize ?? MAX_MESSAGE_SIZE
maxMessageSize: init.dataChannelOptions?.maxMessageSize ?? init.maxDataSize
}
this.maxDataSize = init.maxDataSize

Expand Down Expand Up @@ -275,7 +282,7 @@ export function createStream (options: WebRTCStreamOptions): WebRTCStream {
return new WebRTCStream({
id: direction === 'inbound' ? (`i${channel.id}`) : `r${channel.id}`,
direction,
maxDataSize: (dataChannelOptions?.maxMessageSize ?? MAX_MESSAGE_SIZE) - PROTOBUF_OVERHEAD,
maxDataSize: (dataChannelOptions?.maxMessageSize ?? MAX_MESSAGE_SIZE) - PROTOBUF_OVERHEAD - VARINT_LENGTH,
dataChannelOptions,
onEnd,
channel,
Expand Down
12 changes: 6 additions & 6 deletions packages/transport-webrtc/test/stream.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import { createStream } from '../src/stream.js'
const mockDataChannel = (opts: { send: (bytes: Uint8Array) => void, bufferedAmount?: number }): RTCDataChannel => {
return {
readyState: 'open',
close: () => {},
addEventListener: (_type: string, _listener: () => void) => {},
removeEventListener: (_type: string, _listener: () => void) => {},
close: () => { },
addEventListener: (_type: string, _listener: () => void) => { },
removeEventListener: (_type: string, _listener: () => void) => { },
...opts
} as RTCDataChannel
}
Expand All @@ -23,7 +23,8 @@ const MAX_MESSAGE_SIZE = 16 * 1024
describe('Max message size', () => {
it(`sends messages smaller or equal to ${MAX_MESSAGE_SIZE} bytes in one`, async () => {
const sent: Uint8ArrayList = new Uint8ArrayList()
const data = new Uint8Array(MAX_MESSAGE_SIZE - 5)
const PROTOBUF_OVERHEAD = 5
const data = new Uint8Array(MAX_MESSAGE_SIZE - PROTOBUF_OVERHEAD)
const p = pushable()

// Make sure that the data that ought to be sent will result in a message with exactly MAX_MESSAGE_SIZE
Expand All @@ -42,8 +43,7 @@ describe('Max message size', () => {
p.end()
await webrtcStream.sink(p)

// length(message) + message + length(FIN) + FIN
expect(length(sent)).to.equal(4)
expect(length(sent)).to.equal(6)

for (const buf of sent) {
expect(buf.byteLength).to.be.lessThanOrEqual(MAX_MESSAGE_SIZE)
Expand Down