From 9396ea6d422d6d8f35f2c898bcf621c4895cd9b9 Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Fri, 9 Mar 2018 21:47:14 +0100 Subject: [PATCH] Fix text input event segfault The text input control_event was initially designed for mapping SDL_TextInputEvent, limited to 32 characters. For simplicity, the copy/paste feature was implemented using the same control_event: it just sends the text to paste. However, the pasted text might have a length breaking some assumptions: - on the client, the event max-size was smaller than the text max-length, - on the server, the raw buffer storing the events was smaller than the max event size. Fix these inconsistencies, and encode the length on 2 bytes, to accept more than 256 characters. Fixes . --- app/src/controlevent.c | 6 ++-- app/src/controlevent.h | 4 +-- app/tests/test_control_event_serialize.c | 25 +++++++++++++-- .../genymobile/scrcpy/ControlEventReader.java | 6 ++-- .../scrcpy/ControlEventReaderTest.java | 31 ++++++++++++++++--- 5 files changed, 57 insertions(+), 15 deletions(-) diff --git a/app/src/controlevent.c b/app/src/controlevent.c index d288cee4..ba52e717 100644 --- a/app/src/controlevent.c +++ b/app/src/controlevent.c @@ -39,9 +39,9 @@ int control_event_serialize(const struct control_event *event, unsigned char *bu // injecting a text takes time, so limit the text length len = TEXT_MAX_LENGTH; } - buf[1] = (Uint8) len; - memcpy(&buf[2], event->text_event.text, len); - return 2 + len; + write16(&buf[1], (Uint16) len); + memcpy(&buf[3], event->text_event.text, len); + return 3 + len; } case CONTROL_EVENT_TYPE_MOUSE: buf[1] = event->mouse_event.action; diff --git a/app/src/controlevent.h b/app/src/controlevent.h index 7c52d6e0..65f965e4 100644 --- a/app/src/controlevent.h +++ b/app/src/controlevent.h @@ -9,8 +9,8 @@ #include "common.h" #define CONTROL_EVENT_QUEUE_SIZE 64 -#define SERIALIZED_EVENT_MAX_SIZE 33 -#define TEXT_MAX_LENGTH 256 +#define TEXT_MAX_LENGTH 300 +#define SERIALIZED_EVENT_MAX_SIZE 3 + TEXT_MAX_LENGTH enum control_event_type { CONTROL_EVENT_TYPE_KEYCODE, diff --git a/app/tests/test_control_event_serialize.c b/app/tests/test_control_event_serialize.c index b983fea9..112ea8b8 100644 --- a/app/tests/test_control_event_serialize.c +++ b/app/tests/test_control_event_serialize.c @@ -35,16 +35,36 @@ static void test_serialize_text_event() { unsigned char buf[SERIALIZED_EVENT_MAX_SIZE]; int size = control_event_serialize(&event, buf); - assert(size == 15); + assert(size == 16); const unsigned char expected[] = { 0x01, // CONTROL_EVENT_TYPE_KEYCODE - 0x0d, // text length + 0x00, 0x0d, // text length 'h', 'e', 'l', 'l', 'o', ',', ' ', 'w', 'o', 'r', 'l', 'd', '!', // text }; assert(!memcmp(buf, expected, sizeof(expected))); } +static void test_serialize_long_text_event() { + struct control_event event; + event.type = CONTROL_EVENT_TYPE_TEXT; + char text[TEXT_MAX_LENGTH]; + memset(text, 'a', sizeof(text)); + event.text_event.text = text; + + unsigned char buf[SERIALIZED_EVENT_MAX_SIZE]; + int size = control_event_serialize(&event, buf); + assert(size == 3 + sizeof(text)); + + unsigned char expected[3 + TEXT_MAX_LENGTH]; + expected[0] = 0x01; // CONTROL_EVENT_TYPE_KEYCODE + expected[1] = 0x01; + expected[2] = 0x2c; // text length (16 bits) + memset(&expected[3], 'a', TEXT_MAX_LENGTH); + + assert(!memcmp(buf, expected, sizeof(expected))); +} + static void test_serialize_mouse_event() { struct control_event event = { .type = CONTROL_EVENT_TYPE_MOUSE, @@ -114,6 +134,7 @@ static void test_serialize_scroll_event() { int main() { test_serialize_keycode_event(); test_serialize_text_event(); + test_serialize_long_text_event(); test_serialize_mouse_event(); test_serialize_scroll_event(); return 0; diff --git a/server/src/main/java/com/genymobile/scrcpy/ControlEventReader.java b/server/src/main/java/com/genymobile/scrcpy/ControlEventReader.java index c1250896..83088b10 100644 --- a/server/src/main/java/com/genymobile/scrcpy/ControlEventReader.java +++ b/server/src/main/java/com/genymobile/scrcpy/ControlEventReader.java @@ -13,8 +13,8 @@ public class ControlEventReader { private static final int SCROLL_PAYLOAD_LENGTH = 16; private static final int COMMAND_PAYLOAD_LENGTH = 1; - private static final int TEXT_MAX_LENGTH = 256; - private static final int RAW_BUFFER_SIZE = 128; + public static final int TEXT_MAX_LENGTH = 300; + private static final int RAW_BUFFER_SIZE = 1024; private final byte[] rawBuffer = new byte[RAW_BUFFER_SIZE]; private final ByteBuffer buffer = ByteBuffer.wrap(rawBuffer); @@ -94,7 +94,7 @@ public class ControlEventReader { if (buffer.remaining() < 1) { return null; } - int len = toUnsigned(buffer.get()); + int len = toUnsigned(buffer.getShort()); if (buffer.remaining() < len) { return null; } diff --git a/server/src/test/java/com/genymobile/scrcpy/ControlEventReaderTest.java b/server/src/test/java/com/genymobile/scrcpy/ControlEventReaderTest.java index 4d9952d0..d2b10ccb 100644 --- a/server/src/test/java/com/genymobile/scrcpy/ControlEventReaderTest.java +++ b/server/src/test/java/com/genymobile/scrcpy/ControlEventReaderTest.java @@ -3,14 +3,15 @@ package com.genymobile.scrcpy; import android.view.KeyEvent; import android.view.MotionEvent; -import org.junit.Assert; -import org.junit.Test; - import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.DataOutputStream; import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.util.Arrays; + +import org.junit.Assert; +import org.junit.Test; public class ControlEventReaderTest { @@ -43,8 +44,8 @@ public class ControlEventReaderTest { DataOutputStream dos = new DataOutputStream(bos); dos.writeByte(ControlEvent.TYPE_TEXT); byte[] text = "testé".getBytes(StandardCharsets.UTF_8); - dos.writeByte(text.length); - dos.write("testé".getBytes(StandardCharsets.UTF_8)); + dos.writeShort(text.length); + dos.write(text); byte[] packet = bos.toByteArray(); reader.readFrom(new ByteArrayInputStream(packet)); @@ -54,6 +55,26 @@ public class ControlEventReaderTest { Assert.assertEquals("testé", event.getText()); } + @Test + public void testParseLongTextEvent() throws IOException { + ControlEventReader reader = new ControlEventReader(); + + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + DataOutputStream dos = new DataOutputStream(bos); + dos.writeByte(ControlEvent.TYPE_TEXT); + byte[] text = new byte[ControlEventReader.TEXT_MAX_LENGTH]; + Arrays.fill(text, (byte) 'a'); + dos.writeShort(text.length); + dos.write(text); + byte[] packet = bos.toByteArray(); + + reader.readFrom(new ByteArrayInputStream(packet)); + ControlEvent event = reader.next(); + + Assert.assertEquals(ControlEvent.TYPE_TEXT, event.getType()); + Assert.assertEquals(new String(text, StandardCharsets.US_ASCII), event.getText()); + } + @Test public void testParseMouseEvent() throws IOException { ControlEventReader reader = new ControlEventReader();