luvit / lit

Toolkit for developing, sharing, and running luvit/lua programs and libraries.
http://lit.luvit.io/
Apache License 2.0
245 stars 58 forks source link

WebSocket decoder bug #210

Closed SinisterRectus closed 7 years ago

SinisterRectus commented 7 years ago

Got this error using coro-wrapper and websocket-codec 3.0:

websocket-codec.lua:72: bad argument #1 to 'lshift' (number expected, got nil)

In coro-wrapper:

local item, newIndex = decode(buffer, index)

In websocket-codec:

local function decode(chunk, index)
  local start = index - 1
  local length = #chunk - start
  if length < 2 then return end
  local second = byte(chunk, start + 2)
  local len = band(second, 0x7f)
  local offset
  if len == 126 then
    if #chunk < 4 then return end
    len = bor(
      lshift(byte(chunk, start + 3), 8), -- line 72
      byte(chunk, start + 4))
    offset = 4
...

It seems that byte(chunk, start + 3) is rarely nil.

Will update with more information when I can.

SinisterRectus commented 7 years ago

Seems to be a result of a split chunk, where only a part of one chunk is found at the end of the current buffer. I'm not sure how this happens or why the newer libraries fail to account for it.

SinisterRectus commented 7 years ago

Some debug info while reading a bad buffer:

#chunk index start length second len
4096 1 0 4096 126 126
4096 228 227 3869 126 126
4096 435 434 3662 126 126
4096 627 626 3470 126 126
4096 799 798 3298 126 126
4096 1010 1009 3087 126 126
4096 1201 1200 2896 126 126
4096 1458 1457 2639 126 126
4096 1629 1628 2468 126 126
4096 1820 1819 2277 126 126
4096 2077 2076 2020 126 126
4096 2268 2267 1829 126 126
4096 2479 2478 1618 126 126
4096 2677 2676 1420 126 126
4096 2848 2847 1249 126 126
4096 2984 2983 1113 126 126
4096 3183 3182 914 126 126
4096 3354 3353 743 126 126
4096 3527 3526 570 126 126
4096 3716 3715 381 126 126
4096 3886 3885 211 126 126
4096 4095 4094 2 126 126

This is definitely a result of there not being enough data in the buffer to read. I noticed in 27a3d962e1fa271df40271c5d4fe1dc9bc62c8c9 that you forgot to account for the still-existing, already-read part of the buffer when checking the length of the chunk to be read. I'm assuming that this is the cause of this issue, so I'll go ahead and open a PR for this.