Bug #2081
closed
WebSocketFace: assertion failed when packet is too large
Added by Junxiao Shi about 10 years ago.
Updated almost 10 years ago.
Description
Snippet to reproduce: (using NDN-JS v0.6 for browser)
var face = new ndn.Face({ host:'example.net', port:9696 });
var n = new ndn.Name();
for (var i = 0; i < 5000; ++i) {
n.append('X');
}
console.log(n.toUri());
face.expressInterest(n, function(){}, function(){});
Expected: NFD either closes the face, or ignores the packet.
Actual: NFD crashes with nfd: ../daemon/face/websocket-face.cpp:102: void nfd::WebSocketFace::handleReceive(const string&): Assertion 'msg.size() <= MAX_NDN_PACKET_SIZE' failed.
- Description updated (diff)
StreamFace would close the connection if packet is too large. WebSocketFace is similar to a StreamFace so it shall do the same.
In my opinion, WebSocket is more closer to UDP than to TCP face. The primary reason we drop "large" packets is to avoid memory allocation for unknown-size packets, and without full processing we would not know the message/NDN packet boundaries. With WebSockets, the library itself is doing allocation (I could be wrong here) and the message boundaries are provided by the library. If we can just limit the allocation size for each websocket message, we should just ignore the large packet sizes.
We need to make a decision on:
- Is it allowed for a single WebSocket message to contain multiple NDN packets?
- Is it allowed for a single NDN packets to span across multiple WebSocket messages?
The decision should be documented.
If both answers are NO, WebSocketFace shall be similar to a datagram face.
Junxiao Shi wrote:
We need to make a decision on:
- Is it allowed for a single WebSocket message to contain multiple NDN packets?
I would answer no to this one. NDN.JS never concatenates two NDN packets together into a single "send()" call. So this should never happen.
- Is it allowed for a single NDN packets to span across multiple WebSocket messages?
The answer to this one is a little more tricky. A single NDN packet separated across multiple WS messages may appear when the tcp-websocket proxy is used. When we built the proxy two years ago, we decided to keep it simple by making it stateless. Therefore the proxy will translate each TCP segment into a separate WS message without reassembling the TCP segments together. (Note that it is valid for an NDN packet to span across multiple TCP segments.) As a result, NDN.JS is required to accept NDN packets that span across multiple WS messages.
The WebSocketFace implementation in NFD never sends a single NDN packet in multiple WS messages. But it is prepared to receive such kind of packets (just like what NDN.JS does).
On client side, I'm not sure whether there is always a 1-to-1 mapping between "send()" calls in JavaScript and the WS messages sent on the wire. I checked the WebSocket API document but couldn't find the clear answer to that. If somehow we can confirm that the browsers never separate one "send()" call into multiple WS messages, and we decide to deprecate tcp-websocket proxy, then we can safely ignore any WS message that contains incomplete NDN packet.
The decision should be documented.
If both answers are NO, WebSocketFace shall be similar to a datagram face.
Any comments about how we proceed with this issue? I would say the best practice for now is to silently drop the packet whose size is greater than MAX_NDN_PACKET_SIZE, instead of crashing the program.
20141103 conference call decides: if a packet is oversize, NFD should produce a WARN-level log, and drop this packet.
The WebSocketFace implementation in NFD never sends a single NDN packet in multiple WS messages. But it is prepared to receive such kind of packets (just like what NDN.JS does).
I just checked the current codebase. It seems that I was wrong: NFD will drop the WebSocket message if it contains incomplete NDN packet. So we should make it clear that WebSocket messages containing incomplete NDN packet is not allowed.
- Status changed from New to Code review
- % Done changed from 0 to 30
- Status changed from Code review to Feedback
Please update NFD Developer Guide to include the decision in note-7.
I couldn't find a proper place to put those discussions. Is there a specific section in nfd-docs that talks about packet size issue?
I couldn't find a proper place to put those discussions. Is there a specific section in nfd-docs that talks about packet size issue?
No. You should add a subsection about WebSocketFace, and describe this decision.
- Status changed from Feedback to Resolved
- % Done changed from 30 to 100
- Status changed from Resolved to Closed
I verified that NFD Developer Guide is updated with accurate information.
Also available in: Atom
PDF