Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sockets: OutOfBand bytes/packets are not supported #8282

Closed
clshortfuse opened this issue Aug 26, 2016 · 8 comments
Closed

sockets: OutOfBand bytes/packets are not supported #8282

clshortfuse opened this issue Aug 26, 2016 · 8 comments
Labels
feature request Issues that request new features to be added to Node.js. net Issues and PRs related to the net subsystem.

Comments

@clshortfuse
Copy link
Contributor

Node v6.3.0
Linux localhost 3.14.0 #1 SMP PREEMPT Thu Aug 25 01:02:56 PDT 2016 x86_64 Intel(R) Core(TM) i7-5500U CPU @ 2.40GHz GenuineIntel GNU/Linux
net

When a TCP client, that connects to a NodeJS server, sends an OutOfBand/Urgent byte, the packet is completely dropped.

Full disclosure, I'm migrating an Android (client) /.NET (server) application that uses OutOfBand/Urgent bytes and have hit a wall. If the client send a packet with OutOfBand flag set, there's no way for me to receive it on the server (nodeJS).

I can understand if the default implementation is to drop them, but there should be an option to allow these packets to come in.

@clshortfuse
Copy link
Contributor Author

clshortfuse commented Aug 26, 2016

ffi to the rescue:

var ffi = require('ffi');
var ref = require('ref');
var lib = new ffi.DynamicLibrary();
var setsockopt = ffi.ForeignFunction(lib.get('setsockopt'), 
  ref.types.int, 
  [ref.types.int, ref.types.int, ref.types.int, ref.refType(ref.types.void), ref.refType(ref.types.uint32)]);

var SOL_SOCKET = 0x1;
var SO_OOBINLINE = 0xA;
setsockopt(socket._handle.fd, 
  SOL_SOCKET, 
  SO_OOBINLINE, 
  ref.alloc(ref.types.bool, true), 
  ref.alloc(ref.types.int, ref.types.bool.size));

My only concern is that SOL_SOCKET and SO_OOBINLINE may differ based on architecture.

@mscdex mscdex added net Issues and PRs related to the net subsystem. feature request Issues that request new features to be added to Node.js. labels Aug 26, 2016
@clshortfuse
Copy link
Contributor Author

Should I open another issue for the fact the packets are dropped? Having them inline would be a feature I can understand, but the packets shouldn't be dropped because of the header. I'm unclear because libuv seems to send it as a another packet and not dropping it, but for some reason nodejs drops them?

@Trott
Copy link
Member

Trott commented Jul 10, 2017

@bnoordhuis, @indutny, @nodejs/streams: Thoughts on this one? (Close? Keep open? Any answer to @clshortfuse's question in the last comment?)

@addaleax
Copy link
Member

I’m not sure about libuv, I guess this would have to be implemented there first. This doesn’t seem like an unreasonable feature, just something where somebody needs to invest their time to actually do it.

@bnoordhuis
Copy link
Member

There is an in-progress pull request (libuv/libuv#1040) that lets uv_poll_t handles poll with POLLPRI in order to receive notifications for OOB data.

Apropos SO_OOBINLINE, libuv sets it on MacOS but that's a mitigation technique for a denial-of-service that platform is (or was) vulnerable to.

@bnoordhuis
Copy link
Member

libuv/libuv#1040 was merged sometime ago so it's now possible - at least in theory - to teach node.js about the UV_PRIORITIZED flag to uv_poll_start().

Having said that, it's something of an uphill battle because node.js currently doesn't use1 or expose uv_poll_start(). (You could use ffi to access it though.)

Unless someone is willing to work on it, I suggest we close this. The fact that this issue has been open for a year without attracting much comment or activity is, I think, a good indicator that it's not much in use. That it wasn't raised until 2016 is too.

1 Except internally in one place.

@mcollina
Copy link
Member

mcollina commented Oct 6, 2017

Closing, feel free to reopen if you want to work on this!

@mcollina mcollina closed this as completed Oct 6, 2017
@iamakulov
Copy link

For the reference, @clshortfuse also created a library to get a crossplatform socket ID: https://github.com/clshortfuse/node-getsockethandleaddress. (Useful with the code from #8282 (comment))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. net Issues and PRs related to the net subsystem.
Projects
None yet
Development

No branches or pull requests

7 participants