Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

udp: make it possible to receive empty udp packets #7885

Closed
wants to merge 6 commits into from

Conversation

txdv
Copy link

@txdv txdv commented Jul 2, 2014

A udp packet can have 0 content.
In that case nread will be equal 0, but addr != NULL.

A udp packet can have 0 content.
In that case nread will be equal 0, but addr != NULL.
callback();
});

client.send(new Buffer(1), 0, 0, server_port, "127.0.0.1", function (err, bytes) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Longer than 80 chars.

@trevnorris
Copy link

@indutny can you verify that the change of

-  if (nread == 0) {
+  if (nread == 0 && addr == NULL) {

is alright?

@txdv
Copy link
Author

txdv commented Jul 2, 2014

@trevnorris saghul confirmed it, look at this pull: joyent/libuv#1350 which contains this commit: txdv/libuv@d17bfc6

By the way, you can just use the test to see for yourself...

@trevnorris
Copy link

@txdv thanks for confirming. Take care of the comments and I'll land it.

@txdv
Copy link
Author

txdv commented Jul 2, 2014

ill squash them, wait

@txdv
Copy link
Author

txdv commented Jul 2, 2014

squashed them, ready for review

@trevnorris
Copy link

Thanks. Merged to a382c9a.

@trevnorris trevnorris closed this Jul 3, 2014
@misterdjules
Copy link

It seems that empty UDP datagrams cannot be received on MacOS X with libuv.

As a result, test/simple/test-dgram-send-empty-buffer.js and test/simple/test-dgram-empty-packet.js fail on MacOS X. See jenkin's reports here and here.

I created a repository with a minimal repro of the issue in plain C/libuv . You can have a look at the README to know how to reproduce the issue. This shows that the issue seems to be in libuv itself, or at a lower level.

I haven't had the time to investigate more, but any help is more than welcome!

@trevnorris
Copy link

@misterdjules Can you file a new issue with this same information? I don't have an OSX machine that I can test this on, and it'll be helpful for being able to triage and assign out to have this fixed.

@misterdjules
Copy link

@trevnorris Done! I'd rather wait for your feedback to dig deeper, in the meantime I'm checking the rest of the tests that are failing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants