search

amqp / rhea

A reactive messaging library based on the AMQP protocol
Apache License 2.0
273 stars 80 forks source link

Regression in PR #382: Handle transfer with no payload #388

Open RMueAZD opened 1 year ago

RMueAZD commented 1 year ago

There seems to be a regression in PR #382 which was fixed in PR #349 (addressed in issue #347): Newly added line 364 in session.js is accessing <undefined>.length inside Buffer.concat() if trailing message frame has no payload.

const data = current.frames.length === 1 ? current.frames[0] : Buffer.concat(current.frames);

Problem seems to originate from line 322:

current.frames.push(frame.payload);

If the message has no payload this appends undefined to frames.

Stack trace (v3.0.1):

node:buffer:545
TypeError: Cannot read properties of undefined (reading 'length')
  at Function.concat (node:buffer:545:19)
  at Incoming.on_transfer (node_modules/rhea/lib/session.js:364:83)
  at Session.on_transfer (node_modules/rhea/lib/session.js:764:19)
  at Connection.<computed> [as on_transfer] (node_modules/rhea/lib/connection.js:855:30)
  at c.dispatch (node_modules/rhea/lib/types.js:946:33)
  at Transport.read (node_modules/rhea/lib/transport.js:117:36)
  at SaslClient.read (node_modules/rhea/lib/sasl.js:344:26)
  at Connection.input (node_modules/rhea/lib/connection.js:567:37)
  at TLSSocket.emit (node:events:527:28)
kohtala commented 1 year ago

Hi. Sorry for the issue. I certainly did not make it on purpose. I see a fix is merged.

To help avoid creating this kind of regression, it would be great to add also test cases for these messages without payload.