search

esp-rs / esp-mbedtls

mbedtls for ESP32 bare-metal
Apache License 2.0
17 stars 7 forks source link

Wrong length used in `sync_receive()` making the buffer pull more data than read. #29

Closed AnthonyGrondin closed 1 month ago

AnthonyGrondin commented 2 months ago

This took quite a while to figure out. At first I realized that we couldn't read single bytes from AsyncConnectedSession::read() by calling it with a single byte slice.

It turns out the issue is with how the bytes are pulled in:
sync_receive(ctx: *mut c_void, buf: *mut c_uchar, len: usize) -> c_int.

By running async_server.rs with loglevel set to debug, and calling connected_session.read() with a buffer of len 489*, the second iteration of the loop freezes and we can notice the following output:

DEBUG - async read called
DEBUG - <<< got 511 bytes from socket
DEBUG - <<< read data from mbedtls
DEBUG - *** sync rcv, len=5
DEBUG - *** pulled 5 bytes from rx-buffer
DEBUG - *** sync rcv, len=506
DEBUG - *** pulled 506 bytes from rx-buffer
DEBUG - <<< mbedtls returned 489
DEBUG - async read called
read error: Unknown

The following line reads 489 bytes which is the correct value as its the size of the buffer, but at the same time, 511 bytes in total get pulled from the buffer. https://github.com/esp-rs/esp-mbedtls/blob/b339fe8430bdca8ea53ed2c89b25e825fe6c5f11/esp-mbedtls/src/lib.rs#L826

Also, unrelated but I think this line should compare rx_buffer, not tx_buffer: https://github.com/esp-rs/esp-mbedtls/blob/b339fe8430bdca8ea53ed2c89b25e825fe6c5f11/esp-mbedtls/src/lib.rs#L875

EDIT: The given values, 489 and 511 are arbritrary, as they depend of the headers. For complete usability, it should be able to read a single byte at a time, without too much additional processing latency.