Project

General

Profile

Bug #2081

WebSocketFace: assertion failed when packet is too large

Added by Junxiao Shi almost 5 years ago. Updated almost 5 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Faces
Target version:
Start date:
10/20/2014
Due date:
% Done:

100%

Estimated time:
2.00 h

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.

History

#1 Updated by Junxiao Shi almost 5 years ago

  • 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.

#2 Updated by Alex Afanasyev almost 5 years ago

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.

#3 Updated by Junxiao Shi almost 5 years ago

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.

#4 Updated by Wentao Shang almost 5 years ago

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.

#5 Updated by Wentao Shang almost 5 years ago

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.

#6 Updated by Junxiao Shi almost 5 years ago

20141103 conference call decides: if a packet is oversize, NFD should produce a WARN-level log, and drop this packet.

#7 Updated by Wentao Shang almost 5 years ago

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.

#8 Updated by Wentao Shang almost 5 years ago

  • Status changed from New to Code review
  • % Done changed from 0 to 30

#9 Updated by Junxiao Shi almost 5 years ago

  • Status changed from Code review to Feedback

Please update NFD Developer Guide to include the decision in note-7.

#10 Updated by Wentao Shang almost 5 years ago

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?

#11 Updated by Junxiao Shi almost 5 years ago

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.

#12 Updated by Wentao Shang almost 5 years ago

  • Status changed from Feedback to Resolved
  • % Done changed from 30 to 100

#13 Updated by Junxiao Shi almost 5 years ago

  • Status changed from Resolved to Closed

I verified that NFD Developer Guide is updated with accurate information.

Also available in: Atom PDF