From 2dbe4da1d95f45212c094b20515c8b69b92ba21a Mon Sep 17 00:00:00 2001 From: Ed Rose Date: Tue, 26 Mar 2024 09:44:43 +0000 Subject: [PATCH] Fix blockwise response logic (#376) * Update blockwise tests to catch edge-cases Updates the blockwise tests to catch the cases where the payload is smaller than 'maxPacket', but the overall packet size after the headers are added exceeds 'maxPacket' which causes an error to be thrown. Also adds checks to ensure that the response code is 2.05 for the tests that require it, so 5.00 errors can't sneak through. * Update automatic blockwise transfer logic Fix for blockwise transfer issues raised in #373 where a coap message could avoid being split into blocks, yet still be larger than the allowable size of 1280 bytes. This applies the recommended maximum payload size from RFC 7252 Section 4.6 of 1024 bytes, and allows it to be reduced in the config parameters as required. The error that this fixes was introduced in d727d430d7c744a516d4c68d156ed6bf8262f7b7 and this attempts to keep the intent behind that commit, whilst fixing the technical flaws that it introduced. The 'IP_MTU' constant has been renamed to 'MAX_PAYLOAD' to more accurately describe what the it represents. The 'maxPacketSize' parameter has also been renamed to 'maxPayloadSize' for the same reason. Note that the renaming of the parameter is a potentially breaking change. * Perform extra checks on CoAP message size This fixes the logic for checking the maximum CoAP message size. The check is actually performed in the coap-packet repository in a default parameter on the `generate()` function, however the default value is not appropriate for all (if any) cases. The maximum size that a CoAP message can be is the IP MTU, minus the IP header and minus the UDP header. The value is not constant across all IP network stacks, so the CoAP specification recommends a maximum of 1152 bytes for cases where it is not known. The only way to know for sure is MTU path discovery, which is way outside of the scope of the project. This commit creates a parameter that allows the max packet size to be adjusted as a server parameter for cases where (for example) the server is running on a 6LoWPAN/Thread network and needs a lower maximum message size. Note that the logic for enforcing the size is just to throw an error and crash the server. However, since the maximum payload size is enforced a situation like that should never occur. --- lib/parameters.ts | 14 +++++++++++--- lib/server.ts | 40 +++++++++++++++++++++++----------------- models/models.ts | 6 ++++-- test/blockwise.ts | 11 +++++++++-- 4 files changed, 47 insertions(+), 24 deletions(-) diff --git a/lib/parameters.ts b/lib/parameters.ts index 0ccd410f..8232d315 100644 --- a/lib/parameters.ts +++ b/lib/parameters.ts @@ -131,9 +131,16 @@ const NON_LIFETIME = 145 const COAP_PORT = 5683 /** - * Default max packet size based on IP MTU. + * Maximum total size of the CoAP application layer message, as + * recommended by the CoAP specification */ -const IP_MTU = 1280 +const MAX_MESSAGE_SIZE = 1152 + +/** + * Default max payload size recommended in the CoAP specification + * For more info see RFC 7252 Section 4.6 + */ +const MAX_PAYLOAD_SIZE = 1024 /* Custom default parameters */ @@ -176,7 +183,8 @@ const p: Parameters = { maxLatency: MAX_LATENCY, nonLifetime: NON_LIFETIME, coapPort: COAP_PORT, - maxPacketSize: IP_MTU, + maxPayloadSize: MAX_PAYLOAD_SIZE, + maxMessageSize: MAX_MESSAGE_SIZE, sendAcksForNonConfirmablePackets, piggybackReplyMs, pruneTimerPeriod diff --git a/lib/server.ts b/lib/server.ts index 4f44beb3..7578c112 100644 --- a/lib/server.ts +++ b/lib/server.ts @@ -149,11 +149,13 @@ class CoAPServer extends EventEmitter { // We use an LRU cache for the responses to avoid // DDOS problems. - // max packet size is 1280 - // 32 MB / 1280 = 26214 - // The max lifetime is roughly 200s per packet. - // Which gave us 131 packets/second guarantee - let maxSize = 32768 * 1024 + // total cache size is 32MiB + // max message size is 1152 bytes + // 32 MiB / 1152 = 29127 messages total + + // The max lifetime is roughly 200s per message. + // Which gave us 145 messages/second guarantee + let maxSize = 32768 * 1024 // Maximum cache size is 32 MiB if (typeof this._options.cacheSize === 'number' && this._options.cacheSize >= 0) { maxSize = this._options.cacheSize @@ -216,7 +218,7 @@ class CoAPServer extends EventEmitter { payload, messageId: packet != null ? packet.messageId : undefined, token: packet != null ? packet.token : undefined - }) + }, parameters.maxMessageSize) if (this._sock instanceof Socket) { this._sock.send(message, 0, message.length, rsinfo.port) @@ -227,7 +229,7 @@ class CoAPServer extends EventEmitter { const url = new URL(proxyUri) const host = url.hostname const port = parseInt(url.port) - const message = generate(removeProxyOptions(packet)) + const message = generate(removeProxyOptions(packet), parameters.maxMessageSize) if (this._sock instanceof Socket) { this._sock.send(message, port, host, callback) @@ -237,7 +239,7 @@ class CoAPServer extends EventEmitter { _sendReverseProxied (packet: ParsedPacket, rsinfo: AddressInfo, callback?: (error: Error | null, bytes: number) => void): void { const host = rsinfo.address const port = rsinfo.port - const message = generate(packet) + const message = generate(packet, parameters.maxMessageSize) if (this._sock instanceof Socket) { this._sock.send(message, port, host, callback) @@ -455,7 +457,7 @@ class CoAPServer extends EventEmitter { const sender = new RetrySend(sock, rsinfo.port, rsinfo.address) try { - buf = generate(packet) + buf = generate(packet, parameters.maxMessageSize) } catch (err) { response.emit('error', err) return @@ -646,13 +648,17 @@ class CoAPServer extends EventEmitter { } } -// maxBlock2 is in formular 2**(i+4), and must <= 2**(6+4) -let maxBlock2 = Math.pow( - 2, - Math.floor(Math.log(parameters.maxPacketSize) / Math.log(2)) -) -if (maxBlock2 > Math.pow(2, 6 + 4)) { - maxBlock2 = Math.pow(2, 6 + 4) +// Max block size defined in the protocol is 2^(6+4) = 1024 +let maxBlock2 = 1024 + +// Some network stacks (e.g. 6LowPAN/Thread) might have a lower IP MTU. +// In those cases the maxPayloadSize parameter can be adjusted +if (parameters.maxPayloadSize < 1024) { + // CoAP Block2 header only has sizes of 2^(i+4) for i in 0 to 6 inclusive, + // so pick the next size down that is supported + let exponent = Math.log2(parameters.maxPayloadSize) + exponent = Math.floor(exponent) + maxBlock2 = Math.pow(2, exponent) } /* @@ -691,7 +697,7 @@ class OutMessage extends OutgoingMessage { // if payload is suitable for ONE message, shoot it out if ( payload == null || - (requestedBlockOption == null && payload.length < parameters.maxPacketSize) + (requestedBlockOption == null && payload.length < maxBlock2) ) { return super.end(payload) } diff --git a/models/models.ts b/models/models.ts index aecf8e67..a0d9927f 100644 --- a/models/models.ts +++ b/models/models.ts @@ -53,7 +53,8 @@ export interface ParametersUpdate { probingRate?: number maxLatency?: number piggybackReplyMs?: number - maxPacketSize?: number + maxPayloadSize?: number + maxMessageSize?: number sendAcksForNonConfirmablePackets?: boolean pruneTimerPeriod?: number } @@ -69,7 +70,8 @@ export interface Parameters { piggybackReplyMs: number nonLifetime: number coapPort: number - maxPacketSize: number + maxPayloadSize: number + maxMessageSize: number sendAcksForNonConfirmablePackets: boolean pruneTimerPeriod: number maxTransmitSpan: number diff --git a/test/blockwise.ts b/test/blockwise.ts index 1abc55af..3dfd9447 100644 --- a/test/blockwise.ts +++ b/test/blockwise.ts @@ -59,12 +59,13 @@ describe('blockwise2', function () { } it('should server not use blockwise in response when payload fit in one packet', function (done) { - const payload = Buffer.alloc(100) // default max packet is 1280 + const payload = Buffer.alloc(100) // default max payload is 1024 request({ port }) .on('response', (res) => { + expect(res.code).to.eq('2.05') let blockwiseResponse = false for (const i in res.options) { if (res.options[i].name === 'Block2') { @@ -82,11 +83,13 @@ describe('blockwise2', function () { }) }) - it('should use blockwise in response when payload bigger than max packet', function (done) { + it('should use blockwise in response when payload bigger than max payload', function (done) { + const payload = Buffer.alloc(1275) // 1275 produces a CoAP message (after headers) > 1280 request({ port }) .on('response', (res) => { + expect(res.code).to.eq('2.05') let blockwiseResponse = false for (const i in res.options) { if (res.options[i].name === 'Block2') { @@ -109,6 +112,7 @@ describe('blockwise2', function () { port }) .on('response', (res) => { + expect(res.code).to.eq('2.05') expect(typeof res.headers.ETag).to.eql('string') // expect(cache.get(res._packet.token.toString())).to.not.be.undefined setImmediate(done) @@ -125,6 +129,7 @@ describe('blockwise2', function () { }) .setOption('Block2', Buffer.of(0x02)) .on('response', (res) => { + expect(res.code).to.eq('2.05') let block2 for (const i in res.options) { if (res.options[i].name === 'Block2') { @@ -184,6 +189,7 @@ describe('blockwise2', function () { }) .setOption('Block2', Buffer.of(0x10)) // request from block 1, with size = 16 .on('response', (res) => { + expect(res.code).to.eq('2.05') expect(res.payload).to.eql(payload.slice(1 * 16, payload.length + 1)) // expect(cache.get(res._packet.token.toString())).to.not.be.undefined setImmediate(done) @@ -201,6 +207,7 @@ describe('blockwise2', function () { }) .setOption('Block2', Buffer.of(0x0)) // early negotation with block size = 16, almost 10000/16 = 63 blocks .on('response', (res) => { + expect(res.code).to.eq('2.05') expect(res.payload).to.eql(payload) // expect(cache.get(res._packet.token.toString())).to.not.be.undefined setImmediate(done)