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
d727d43 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

@@ -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

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)
         }

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

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)