MylesBorins / node-osc

Open Sound Control protocol library for Node.js
GNU Lesser General Public License v3.0
435 stars 72 forks source link

Cannot read properties of undefined (reading 'elements') #104

Closed alex-Arc closed 1 year ago

alex-Arc commented 1 year ago

If the incoming message is malformed or from some senders has no args then line 34 in Server.mjs seams to throw if the if else statement is moved inside the try we can catch it instead of throwing image

but the real issue seams to stem from decode.mjs where the case of a successful decode but without oscType will cause a void return

MylesBorins commented 1 year ago

@alex-Arc can you share some reproducible examples of this behaviour and what would be expected?

It would appear that in the scenarios you are referencing that decoded itself is malformed and then causing this.emit to throw... which I'm unable to reproduce.

Before moving things around I want to make sure I understand the edge case and am handling the error appropriately

alex-Arc commented 1 year ago

hi @MylesBorins I'm happy to just to sanity check this should be a good packet

plain:
/ontime/change/aa42f/title [no args]

ASCII:
/ontime/change/aa42f/title\00\00,\00\00\00

HEX:
2F 6F 6E 74 69 6D 65 2F 63 68 61 6E 67 65 2F 61 61 34 32 66 2F 74 69 74 6C 65 00 00 2C 00 00 00

if the packet gets malformed in transit or is sent by a program like TouchDesigner the trailing 00 terminators for the args are missing

ASCII:
/ontime/change/aa42f/title\00\00

HEX:
2F 6F 6E 74 69 6D 65 2F 63 68 61 6E 67 65 2F 61 61 34 32 66 2F 74 69 74 6C 65 00 00 
MylesBorins commented 1 year ago

@alex-Arc do you have a way for me to reproduce this bad packet being sent?

alex-Arc commented 1 year ago

I use https://packetsender.com/

alex-Arc commented 1 year ago

@MylesBorins Hey I set up a fuzzing test https://github.com/alex-Arc/node-osc-test and it immediately crashed but adding an final else will catch everything and even better setting the strict flag on fromBuffer will passe up good error messages that is then passes to the event emitter on error image

MylesBorins commented 1 year ago

Thanks for all the details. I've shipped a fix and am releasing a new version right now.