ControlMessage parseInjectTouchEvent expects the wrong size of the incoming buffer

[vyagushe]

When scrcpy is used through the poor wifi connection you can dump into the following crash:

[server] ERROR: Exception on thread Thread[Thread-1,5,main]
java.nio.BufferUnderflowException
	at java.nio.Buffer.nextGetIndex(Buffer.java:493)
	at java.nio.HeapByteBuffer.getShort(HeapByteBuffer.java:258)
	at com.genymobile.scrcpy.ControlMessageReader.parseInjectTouchEvent(ControlMessageReader.java:134)
	at com.genymobile.scrcpy.ControlMessageReader.next(ControlMessageReader.java:64)
	at com.genymobile.scrcpy.DesktopConnection.receiveControlMessage(DesktopConnection.java:108)
	at com.genymobile.scrcpy.Controller.handleEvent(Controller.java:76)
	at com.genymobile.scrcpy.Controller.control(Controller.java:67)
	at com.genymobile.scrcpy.Server$1.run(Server.java:47)
	at java.lang.Thread.run(Thread.java:764)

You can easily reproduce on Linux by using netem

ETH_DEVICE=eth0
sudo tc qdisc del dev $ETH_DEVICE root
sudo tc qdisc add dev $ETH_DEVICE root handle 1:0 netem delay 300ms reorder 25% 50%
sudo tc qdisc add dev $ETH_DEVICE parent 1:1 handle 10: tbf rate 64kbit buffer 1600 limit 3000
sudo tc -s qdisc ls dev $ETH_DEVICE

To make reproduction easier I decreased the raw buffer size to 256:

--- a/server/src/main/java/com/genymobile/scrcpy/ControlMessageReader.java
+++ b/server/src/main/java/com/genymobile/scrcpy/ControlMessageReader.java
@@ -16,7 +16,7 @@ public class ControlMessageReader {
 
     public static final int TEXT_MAX_LENGTH = 300;
     public static final int CLIPBOARD_TEXT_MAX_LENGTH = 4093;
-    private static final int RAW_BUFFER_SIZE = 1024;
+    private static final int RAW_BUFFER_SIZE = 256;

I narrowed down the reason. The server expects the number of remaining bytes in the buffer less than actual touch event size:

app/src/control_msg.c writes 28 bytes

        case CONTROL_MSG_TYPE_INJECT_TOUCH_EVENT:
            buf[1] = msg->inject_touch_event.action;
            buffer_write64be(&buf[2], msg->inject_touch_event.pointer_id);
            write_position(&buf[10], &msg->inject_touch_event.position);
            uint16_t pressure =
                to_fixed_point_16(msg->inject_touch_event.pressure);
            buffer_write16be(&buf[22], pressure);
            buffer_write32be(&buf[24], msg->inject_touch_event.buttons);
            return 28;

server/src/main/java/com/genymobile/scrcpy/ControlMessageReader.java excepts more than 21 bytes

    @SuppressWarnings("checkstyle:MagicNumber")
    private ControlMessage parseInjectTouchEvent() {
        if (buffer.remaining() < INJECT_TOUCH_EVENT_PAYLOAD_LENGTH) {
            return null;
        }

[rom1v]

Thank you for your report.

I think it's a duplicate of #1245, fixed by 89d1602 and tested by 3504c00.

Could you please test dev branch, and confirm it is fixed?

Thank you.

[vyagushe]

This is a duplicate. It is fixed. I already checked this.

[vyagushe]

Don't you consider moving to protobuf?

[rom1v]

No, currently, I don't.

I agree that the manual serialization/deserialization in scrcpy is not great (and error-prone), but protobuf brings its own set of problems (random link), and I think it would add too much complexity for the benefit.