| < Previous by Date | Date Index | Next by Date > |
| < Previous in Thread | Thread Index | Next in Thread > |
On Mar 25, 2013 12:56 AM, "Daniel Pocock" <daniel@xxxxxxxxxxxxx> wrote:
>
>
>
> On 25/03/13 04:05, Byron Campen wrote:
> > Ok, the code that handles buffer allocation for the body runs from
> > ConnectionBase.cxx:303-386. Here's the gist of it:
> >
> > - Parse the Content-Length header, or drop.
>
> http://tools.ietf.org/html/rfc3261#section-20.14
>
> Content-Length is actually optional for UDP and for WebSockets, because
> they both have their own way of specifying the size of a message unit:
>
> http://tools.ietf.org/html/draft-ietf-sipcore-sip-websocket-08#section-5.1
>
Yep, as far as I know the udp code doesn't require it. Should check though.
> This has two consequences:
>
> a) We should not assume it is always there
>
> b) We should add a Content-Length header when necessary (e.g. proxying
> from WebSocket to TLS or TCP)
>
We always add it on send, I think.
> If this appears undesirable, now is the time to comment on the draft.
>
>
> > - Perform sanity-check on value of Content-Length, or drop.
>
> I see 10485760 hardcoded there now (10MB), I would propose:
> - making it configurable
> - WebSocket code will use the same value when screening the size in a
> payload size header
>
Yeah, definitely should be configurable.
> > - If Content-Length indicates that we have already read the entire body in
> > a prior read (ie; while reading headers), do a little buffer management so
> > that the next message is preserved (we've already read at least part of
> > it), call SipMessage::setBody(), and move the message along.
> > - Otherwise, alloc a buffer of either ChunkSize or half again the size of
> > the body read so far, whichever is _bigger_, up to a maximum of the
> > Content-Length.
> >
> > As more bytes of the body come in, whenever we run out of space, we (sort
> > of) repeat the logic in the last step above (see ConnectionBase.cxx:503).
> >
> > In no case is ChunkSize acting as an upper bound on the size of a buffer
> > allocation for a body. Initially it is used as a lower bound, though.
>
> For traditional streams, I agree, it is not so bad. It is the way that
> it has been slightly misused by the WebSocket code that leads to problems.
>
> > In regard to the logging above, I've looked again at UdpTransport since I
> > was not seeing any of the other logging I would expect to, but it seems
> > that the debug logging in there has been commented out. I would bet you $10
> > that what you just saw was a truncated UDP datagram come in due to
> > fragmentation; the Content-Length says there are 4657 bytes of body, but
> > the datagram ends 866 bytes after the headers.
>
> Nathan's example was for WebSocket messages (not UDP), and it was using
> the latest version of b-webrtc, which no longer tries to rearrange
> mBuffer and force it back into preparseNewBytes (see the changes I made
> about a week ago)
> http://svn.resiprocate.org/viewsvn/resiprocate/branches/b-webrtc/resip/stack/ConnectionBase.cxx?r1=10045&r2=10046&diff_format=l
>
> In the WebSocket code, ChunkSize has been used as a limit on buffer
> size, and this is one of the main things I need to resolve
>
On a mobile again, so difficult to look for myself right now.
Best regards,
Byron Campen