webui-dev / webui

Use any web browser or WebView as GUI, with your preferred language in the backend and modern web technologies in the frontend, all in a lightweight portable library.
https://webui.me
MIT License
2.63k stars 157 forks source link

Civetweb Issue #243

Closed hassandraga closed 9 months ago

hassandraga commented 9 months ago

For some reason, data over 97.6Kb (100000 bytes) may have data loss because Civetweb has an issue. That makes the WebUI-protocol-header have incorrect data size, so it crashes (overflow) if the user accesses the most significant byte (ends). However, WebUI detects it and ignores the request so it won't crash now. But we should report this to Civetweb.

Most users won't need to send +97Kb of data, but still, we need to fix this in Civetweb.

UI:

const big_arr = new Uint8Array(100000);
webui.call('Func', big_arr);

Backend logs:

[Core] [Thread 1] _webui_receive_thread() -> No enough data received. Expected 100000 bytes, received 61886 bytes.
fibodevy commented 9 months ago

_webui_ws_data_handler is called 2 times, data comes in 2 packets

If it comes in 1 packet, 100k bytes = 100021 bytes (with header) If it comes in multiple packets, their sum is exactly the same, 100021

Notice another packet is coming while first packet is printing

yuf6yc7sEp

IDK but maybe its how CivetWeb works? Maybe WebUI should handle multiple incoming packets?

A fix would be:

fibodevy commented 9 months ago

Max single packet size for 100% success ratio is 65535

65535 - header - funcname - null - datasize (as string) - null

func name "100k", length 4 "655XX" (data length) as string, length 5

var big_arr = new Uint8Array(65535-8-4-1-5-1);
webui.call('100k', big_arr);

100% success ratio

Add 1 byte

var big_arr = new Uint8Array(65535-8-4-1-5-1+1);
webui.call('100k', big_arr);

50% success

I would do what I proposed in previous post, dont wait for civet to fix it. Just few more lines of code 🙃

hassandraga commented 9 months ago

I double-checked, and I don't see multiple requests in one click. Are you sure this is not two clicks?

1 Click: [Core] _webui_receive([1], [1], [100037])... -> Sucess 1 Click: [Core] _webui_receive([1], [1], [30963])... -> Fails 1 Click: [Core] _webui_receive([1], [1], [61926])... -> Fails 1 Click: [Core] _webui_receive([1], [1], [92881])... -> Fails 1 Click: [Core] _webui_receive([1], [1], [100037])... -> Sucess

However, simply saving data will only work in some scenarios. I'm planning to solve this by adding a new command WEBUI_CMD_CHUNK, JavaScript bridge will break big data into chunks, and WebUI will save it until all chunks are received and then released... but this mechanism has challenges like UI that have other JavaScript workers/threads that can send regular data in the same time, Or simply JavaScript failed to send all chunks as promised, so we may need to add a timer... all those things will make WebUI slower, and to be honest, I'm happy with the 10ms response speed, and I don't want to break it 🤞

What confuses me is that the first send always works fine... Civetweb should always send in multiple chunks or always in one chunk. I would suggest that we wait until Civetweb responds so we can solve this and keep WebUI as fast as possible.

fibodevy commented 9 months ago

I double-checked, and I don't see multiple requests in one click. Are you sure this is not two clicks?

Pretty sure, here is screenshot of Wireshark, you can see 3 packets

eHokJgm6OC

fibodevy commented 9 months ago

With this little mod you will see it comes in multiple packets

webui.c:5561

    #ifdef WEBUI_LOG
        printf("[Core]\t\t_webui_ws_data_handler()...\n");
        printf("[Core]\t\t-----------------------------------------------------\n");
        printf("[Core]\t\t data size = %zu \n", datasize);
        printf("[Core]\t\t-----------------------------------------------------\n");
    #endif
fibodevy commented 9 months ago

However, simply saving data will only work in some scenarios.

Right, if a different packet will come in between big chunked data it will mess it up, so lets wait for Civet's response

hassandraga commented 9 months ago

Thank you @fibodevy for the testing and troubleshooting. Wireshark will show you packets between the browser and Civetweb. And probably Civetweb merges the packets into one, then sends it to the WebUI callback in one packet or something else.

If you confirm that this is one click, then that means Civetweb acts differently on my machine, as I can confirm that WebUI receives only one packet per click. I see one _webui_ws_data_handler() per click.

fibodevy commented 9 months ago

Perhaps its OS+browser dependent, Im testing on Win10 + Chrome

Here is a video of how it looks in my setup

https://github.com/webui-dev/webui/assets/21068718/d4f22cc3-205a-42a0-bdff-60c3d454f05d

Wireshark will show you packets between the browser and Civetweb. And probably Civetweb merges the packets into one, then sends it to the WebUI callback in one packet or something else.

Wireshark SS was just to show the 100k buffer is split, what Civet is doing with it I dont know, but the fact is _webui_ws_data_handler is actually called multiple times, and the datasize is just like the actual packet size in Wireshark

hassandraga commented 9 months ago

Based on their response, we should split the data by ourselves.

hassandraga commented 9 months ago

Fixed.