Bug #2081: WebSocketFace: assertion failed when packet is too large - NFD - NDN project issue tracking system

Actions

Copy link

Bug #2081

closed

WebSocketFace: assertion failed when packet is too large

Added by Junxiao Shi about 10 years ago. Updated almost 10 years ago.

Status:

Closed

Priority:

Normal

Assignee:

Wentao Shang

Category:

Faces

Target version:

v0.3

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.

Actions

Copy link

Updated by Junxiao Shi about 10 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.

Actions

Copy link

Updated by Alex Afanasyev about 10 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.

Actions

Copy link

Updated by Junxiao Shi about 10 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.

Actions

Copy link

Updated by Wentao Shang about 10 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.

Actions

Copy link

Updated by Wentao Shang about 10 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.

Actions

Copy link

Updated by Junxiao Shi about 10 years ago

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

Actions

Copy link

Updated by Wentao Shang about 10 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.

Actions

Copy link