Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase max message size restriction #157

Closed
kliment-slice opened this issue May 17, 2024 · 7 comments · Fixed by #158
Closed

Increase max message size restriction #157

kliment-slice opened this issue May 17, 2024 · 7 comments · Fixed by #158
Labels

Comments

@kliment-slice
Copy link
Contributor

self.max_message_size = 512

For example, when sending a JWT token along with a message, this size ends up too small and results in the ValueError:
raise ValueError(
f"""Total size for message {id} exceeds the allocation limit allowed.
Maximum size = {self.max_message_size},
Received size = {total_size}."""

Proposal: increase e.g. self.max_message_size = 3000 to accommodate.

@jourdain
Copy link
Collaborator

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

@jourdain
Copy link
Collaborator

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
Copy link

awoimbee commented May 17, 2024

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
Copy link
Collaborator

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

@awoimbee
Copy link

I think:

  • the size should be configurable (just store it in a global MAX_HELLO_MESSAGE_SIZE so users can set it to what they want)
  • if we bump the size, 8KB would be nice (it's what most HTTP proxies allow for HTTP headers, in which JWTs are usually sent)

@jourdain
Copy link
Collaborator

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
Copy link
Collaborator

🎉 This issue has been resolved in version 2.0.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants