search

debug-ito / AnyEvent-WebSocket-Server

WebSocket server for AnyEvent
7 stars 2 forks source link

max_payload_size #3

Closed rp3u closed 7 years ago

rp3u commented 7 years ago

Are there any ways to deal with it?

EV: error in callback (ignoring): Payload is too big. Send shorter messages or increase max_payload_size at /usr/local/lib/perl5/site_perl/5.16/Protocol/WebSocket/Frame.pm line 250.

plicease commented 7 years ago

I may be able to provide an interface to max_payload_size. It would probably be a good idea to have an event for this instead of dying in the event loop too. Is the frame that is "too big" on the client or the server side?

rp3u commented 7 years ago

The Server side. Clients are browsers.

debug-ito commented 7 years ago

@plicease or, maybe we should set max_payload_size as big as it is allowed by the protocol (RFC 6455). I don't know why this value is limited to such a small value (backward-compatibility for older version of WebSocket??)

Another solution is to implement fragmentation ( https://tools.ietf.org/html/rfc6455#section-5.4 ), but this requires a lot of work, I guess..

debug-ito commented 7 years ago

see also: https://github.com/vti/protocol-websocket/issues/24

max_payload_size is not documented, but it seems that we can use it.

rp3u commented 7 years ago

To not suspend my work I increased the default value in Frame.pm Of course, it is very dirty fix

plicease commented 7 years ago

max is 64bit with most significant set to 0? Is that 9223372036854775807? Are we going to have problems on Perls with a 32bit UV?

I can't see a reason not to set it to the max for send. For receive having a lower max may be desirable, so I think there should be an option for that.

Fragmentation is okay too, but I won't have the time to implement it.

plicease commented 7 years ago

I think this will do the trick: https://github.com/plicease/AnyEvent-WebSocket-Client/commit/602fbc22b6ae2ce47b9e94cb8579ce13c9868da4 Feedback encouraged.

plicease commented 7 years ago

Tested on Debian 64bit and CentOS 5 32bit (the latter with UV=4) so I don't think the UV size doesn't seem to make any sense difference.

debug-ito commented 7 years ago

@plicease thanks for the quick response. I agree with you that it makes sense to limit size of received frames. https://github.com/plicease/AnyEvent-WebSocket-Client/commit/602fbc22b6ae2ce47b9e94cb8579ce13c9868da4 looks good to me.

However, even though it was I who suggested setting very big number to max_payload_size by default, now I'm a bit worried that it may cause problems in some platforms, like you pointed out. So, maybe it's better to have max_payload_size_sent and max_payload_size_received, making both directions configurable.

plicease commented 7 years ago

I don't think adding an attribute to work around platforms where the library might be broken is the appropriate. It makes the API more complicated, and it should be the job of the library authors + test suite + cpantesters and such to isolate such issues and correct them rather than provide an interface on the off chance that end user developers needs it for a work around. In that event they should be able to patch the library anyway, as @rp3u has done. Ideally Protocol::WebSocket would have an option to not check the payload size.

plicease commented 7 years ago

The maintainer of Protocol::WebSocket has in principal agreed to make max_protocol_size optional, and I have a PR to make it so. Does this address your concerns?

debug-ito commented 7 years ago

Thanks again for quick work. Now there's no problem for me.

After you publish your new AnyEvent::WebSocket::Client, I'll publish a new Server and close this issue.

plicease commented 7 years ago

On its way to CPAN now!

debug-ito commented 7 years ago

Released AnyEvent::WebSocket::Server 0.09, which has option max_payload_size. https://metacpan.org/release/TOSHIOITO/AnyEvent-WebSocket-Server-0.09