Kitware / wslink

Python/JavaScript library for communicating over WebSocket
https://kitware.github.io/wslink/
BSD 3-Clause "New" or "Revised" License
83 stars 27 forks source link

Increase max message size restriction #157

Closed kliment-slice closed 6 months ago

kliment-slice commented 6 months ago

https://github.com/Kitware/wslink/blob/35083905012575ab5f854d9f72a2d84ee26791af/python/src/wslink/chunking.py#L90 For example, when sending a JWT token along with a message, this size ends up too small and results in the ValueError: https://github.com/Kitware/wslink/blob/35083905012575ab5f854d9f72a2d84ee26791af/python/src/wslink/chunking.py#L106-L109 Proposal: increase e.g. self.max_message_size = 3000 to accommodate.

jourdain commented 6 months ago

Are you sure? Once the client get authorized, we bump that size to 4GB.

jourdain commented 6 months ago

Or is it because your token/secret make that initial message pass the 512 limit? Do you know the actual size you are getting?

awoimbee commented 6 months ago

Hi, When we connect to the wslink server we use: WSLinkClient.connect(config). Where config is { sessionURL: serverUrl, secret: JwtToken }.

A standard JWT token makes us go above the 512 limit (the hello message is around ~2.4KB). A big JWT can be >6KB.

Example:

ValueError: Total size for message 2288169018 exceeds the allocation limit allowed.
    Maximum size = 3000,
    Received size = 5117.
00000000: eb6d 7402 0000 0000 fd13 0000 85a6 7773  .mt...........ws
00000001: 6c69 6e6b a331 2e30 a269 64ab 7379 7374  link.1.0.id.syst
00000002: 656d 3a63 303a 30a6 6d65 7468 6f64 ac77  em:c0:0.method.w
00000003: 736c 696e 6b2e 6865 6c6c 6fa4 6172 6773  slink.hello.args
00000004: 9181 a673 6563 7265 74da 13b5 6579 4a68  ...secret...eyJh
00000005: 6247 6369 4f69 4a53 557a 4931 4e69 4973  bGciOiJSUzI1NiIs
00000006: 496e 5235 6343 4967 4f69 4169 536c 6455  InR5cCIgOiAiSldU
00000007: 4969 7769 6132 6c6b 4969 4136 4943 4979  Iiwia2lkIiA6ICIy
00000008: 4e55 497a 5757 6870 5956 4a32 6431 646d  NUIzWWhpYVJ2d1dm
00000009: 4e57 6b74 5657 5632 5345 3577 5a58 6330  NWktVWV2SE5wZXc0
0000000a: 5256 5933 5646 6831 4f48 6c32 4e46 567a  RVY3VFh1OHl2NFVz
0000000b: 646d 5233 5530 315a 496e 302e 6579 4a6c  dmR3U01ZIn0.eyJl
[...]
0000013e: 3537 3973 302d 7647 714f 3757 6149 6178  579s0-vGqO7WaIax
0000013f: 5f4f 696e 4875 7671 6b7a 4b7a 7a31 6471  _OinHuvqkzKzz1dq
00000140: 77a6 6b77 6172 6773 80                   w.kwargs.
jourdain commented 6 months ago

Ok thanks, I just wanted to double check you were using the library in the way it was intended to.

awoimbee commented 6 months ago

I think:

jourdain commented 6 months ago

Agree and you can see my comment on the associated PR to handle that. To some extent, I don't want to bump the default since it is working fine with reasonably long token.

jourdain commented 6 months ago

:tada: This issue has been resolved in version 2.0.3 :tada:

The release is available on:

Your semantic-release bot :package::rocket: