fix: Adding validation for raw data size parsing

trimsock.gd/.gitignore

@@ -3,3 +3,5 @@
 
 # Only for local use
 sh/ensure-uids.sh
+
+.vscode/

trimsock.gd/addons/trimsock.gd/line_parser.gd

@@ -34,6 +34,9 @@ func parse(p_line: String) -> TrimsockCommand:
 	return command
 
 func read_name() -> String:
+	if is_eol():
+		return ""
+
 	if chr() == "\"":
 		return read_quoted()
 	else:

trimsock.gd/addons/trimsock.gd/line_reader.gd

@@ -7,10 +7,16 @@ var at := 0
 var is_quote := false
 var is_escape := false
 
+func reset() -> void:
+	buffer.clear()
+	at = 0
+	is_quote = false
+	is_escape = false
+
 func ingest(data: PackedByteArray) -> Error:
 	var new_size := buffer.size() + data.size()
 	if new_size > max_size:
-		buffer.clear()
+		reset()
 		return ERR_OUT_OF_MEMORY
 
 	buffer.append_array(data)
@@ -30,20 +36,26 @@ func read_text() -> String:
 	return ""
 
 func has_data(size: int) -> bool:
-	return buffer.size() >= size
+	return buffer.size() > size
 
-func read_data(size: int) -> PackedByteArray:
+func read_data(size: int) -> Array:
 	assert(has_data(size), "Trying to read more bytes than available!")
 
 	# Grab result
 	var result := buffer.slice(0, size)
-	buffer = buffer.slice(size)
+	var is_terminated := String.chr(buffer[size]) == "\n"
+
+	buffer = buffer.slice(size + 1)
+	at = 0
 
 	# Reset flags
 	is_escape = false
 	is_quote = false
 
-	return result
+	if not is_terminated:
+		return [ERR_PARSE_ERROR, PackedByteArray()]
+
+	return [OK, result]
 
 func chr() -> String:
 	return String.chr(buffer[at])

trimsock.gd/addons/trimsock.gd/reactor.gd

@@ -37,6 +37,8 @@ var _id_generator: TrimsockIDGenerator = RandomTrimsockIDGenerator.new(12)
 signal on_attach(source: Variant)
 ## Emitted when a known source is detached from the reactor
 signal on_detach(source: Variant)
+## Emitted when data received from a source can't be parsed
+signal on_ingest_error(source: Variant, error: Error, message: String)
 
 
 ## Poll all sources and process incoming data
@@ -48,6 +50,8 @@ func poll() -> void:
 		while true:
 			var command := reader.read()
 			if not command:
+				if reader.last_error != OK:
+					on_ingest_error.emit(source, reader.last_error, reader.last_error_message)
 				break
 
 			_handle(command, source)
@@ -155,7 +159,11 @@ func _write(target: Variant, command: TrimsockCommand) -> void:
 func _ingest(source: Variant, data: PackedByteArray) -> Error:
 	assert(_readers.has(source), "Ingesting data from unknown source! Did you call `attach()`?")
 	var reader := _readers[source] as TrimsockReader
-	return reader.ingest_bytes(data)
+	var error := reader.ingest_bytes(data)
+	if error != OK:
+		on_ingest_error.emit(source, error, reader.last_error_message)
+
+	return error
 
 func _handle(command: TrimsockCommand, source: Variant) -> void:
 	var xchg := _get_exchange_for(command, source)

trimsock.gd/addons/trimsock.gd/reader.gd

@@ -9,31 +9,62 @@ class_name TrimsockReader
 ## [br][br]
 ## After ingestion, call [method read] to extract commands from the ingested
 ## data. Data that is parsed is immediately freed from the internal buffer.
+## [br][br]
+## If the incoming data is malformed, it is discarded and the error is reported
+## in [member last_error]. The reader continues parsing with the data following
+## the offending command.
+
 
+## Upper limit on the internal buffer's size
+##
+## Ingested data is buffered until [method read] can extract a full command
+## from it. If this size limit is exceeded, the buffer's contents are discarded
+## and [constant ERR_OUT_OF_MEMORY] is reported.
+var max_size: int:
+	get:
+		return _line_reader.max_size
+	set(value):
+		_line_reader.max_size = value
+
+var last_error: Error = OK
+## Description of [member last_error]
+var last_error_message := ""
 
 var _line_reader: _TrimsockLineReader = _TrimsockLineReader.new()
 var _line_parser: _TrimsockLineParser = _TrimsockLineParser.new()
 var _queued_raw: TrimsockCommand = null
+var _queued_raw_size := -1
 
 ## Ingest incoming text
 ## [br][br]
 ## Returns [constant OK] on success, or [constant ERR_OUT_OF_MEMORY] if the
 ## internal buffer can't store the data.
 func ingest_text(text: String) -> Error:
-	return _line_reader.ingest(text.to_utf8_buffer())
+	return ingest_bytes(text.to_utf8_buffer())
 
 ## Ingest incoming binary data
 ## [br][br]
 ## Returns [constant OK] on success, or [constant ERR_OUT_OF_MEMORY] if the
 ## internal buffer can't store the data.
 func ingest_bytes(bytes: PackedByteArray) -> Error:
-	return _line_reader.ingest(bytes)
+	_clear_error()
+
+	var error := _line_reader.ingest(bytes)
+	if error != OK:
+		_dequeue_raw()
+		_set_error(error, "Buffer overflow! Can't ingest %d bytes without exceeding %d!" \
+			% [bytes.size(), max_size])
+
+	return error
 
 ## Try and extract a command from the ingested data
 ## [br][br]
 ## Returns a parsed command, or [code]null[/code] if no command is available
-## yet.
+## yet. Malformed data is rejected by returning [code]null[/code] and setting
+## [member last_error].
 func read() -> TrimsockCommand:
+	_clear_error()
+
 	var command := _pop()
 	if command:
 		_TrimsockConventions.apply(command)
@@ -42,17 +73,24 @@ func read() -> TrimsockCommand:
 func _pop() -> TrimsockCommand:
 	# We read a raw command earlier, waiting to have enough data
 	if _queued_raw:
-		var data_size := int(_queued_raw.text)
-		if not _line_reader.has_data(data_size):
+		if not _line_reader.has_data(_queued_raw_size):
+			return null
+
+		var raw_command := _queued_raw
+		var raw_size := _queued_raw_size
+		var read_result := _line_reader.read_data(raw_size)
+
+		_dequeue_raw()
+
+		if read_result[0] != OK:
+			_set_error(read_result[0], "Expected newline after %d bytes of raw data!" % [raw_size])
 			return null
 
-		_queued_raw.raw = _line_reader.read_data(data_size)
-		_queued_raw.text = ""
-		_queued_raw.chunks.clear()
+		raw_command.raw = read_result[1]
+		raw_command.text = ""
+		raw_command.chunks.clear()
 
-		var result := _queued_raw
-		_queued_raw = null
-		return result
+		return raw_command
 
 	# No queued command, try to read a new one
 	var line := _line_reader.read_text()
@@ -61,11 +99,42 @@ func _pop() -> TrimsockCommand:
 
 	var command := _line_parser.parse(line)
 	if command.is_raw:
+		var data_size := _parse_data_size(command.text)
+		if data_size < 0:
+			_set_error(ERR_INVALID_DATA, "Invalid raw command size: \"%s\"!" % [command.text])
+			return null
+
 		# Command is raw, we'll keep it in the queue until we read the binary
 		# data for it
 		_queued_raw = command
+		_queued_raw_size = data_size
 
 		# Try getting it immediately, in case we already have the data in buffer
 		return _pop()
 
 	return command
+
+# Parse the data size of a raw command or return -1 if it's invalid
+func _parse_data_size(text: String) -> int:
+	if not text.is_valid_int():
+		return -1
+
+	var size := text.to_int()
+
+	# The data and its terminating newline must both fit in the buffer
+	if size < 0 or size >= max_size:
+		return -1
+
+	return size
+
+func _dequeue_raw() -> void:
+	_queued_raw = null
+	_queued_raw_size = -1
+
+func _clear_error() -> void:
+	last_error = OK
+	last_error_message = ""
+
+func _set_error(error: Error, message: String) -> void:
+	last_error = error
+	last_error_message = message

trimsock.gd/tests/reactor.test.gd

@@ -99,3 +99,21 @@ func suite():
 		expect(reactor.outbox[0].command.is_stream(), "Command was not a stream!")
 		expect_not_empty(reactor.outbox[0].command.exchange_id, "Stream ID was empty!")
 	)
+
+	test("should keep parsing after an unsatisfiable raw command", func():
+		var errors := []
+		reactor.on_ingest_error.connect(func(source, error, message): errors.append(error))
+
+		var commands := []
+		reactor.on("command", func(cmd, xchg): commands.append(cmd))
+
+		# The blank line terminating the headers parses as a raw command with no size
+		reactor.ingest_text(some_source, "GET / HTTP/1.1\r\nHost: x\r\n\r\n")
+		reactor.poll()
+
+		reactor.ingest_text(some_source, "command foo\n")
+		reactor.poll()
+
+		expect_not_empty(errors, "No errors reported!")
+		expect_not_empty(commands, "No commands handled!")
+	)

trimsock.gd/tests/reader.test.gd

@@ -19,3 +19,134 @@ func suite():
 		expect_true(command.is_raw)
 		expect_equal(command.raw, "a\ncd".to_utf8_buffer())
 	)
+
+	define("raw data terminator", func():
+		test("should read raw message split at its terminator", func():
+			reader.ingest_text("\rcommand 4\n1234")
+			expect_null(reader.read(), "Command was read without its terminator!")
+
+			reader.ingest_text("\n")
+			var command := reader.read()
+
+			expect_not_null(command)
+			expect_equal(command.raw, "1234".to_utf8_buffer())
+		)
+
+		test("should not emit a command for the terminator", func():
+			reader.ingest_text("\rcommand 4\n1234\ncommand after\n")
+
+			expect_equal(read_names(), ["command", "command"])
+		)
+
+		test("should reject raw data with a malformed terminator", func():
+			reader.ingest_text("\rcommand 4\n1234X\n")
+
+			expect_null(reader.read(), "Command was read!")
+			expect_not_equal(reader.last_error, OK, "No error was reported!")
+		)
+
+		test("should keep parsing after a malformed terminator", func():
+			# The malformed terminator is consumed in its place, so parsing
+			# resumes on the next line
+			reader.ingest_text("\rcommand 4\n1234Xcommand foo\n")
+			expect_null(reader.read(), "Command was read!")
+
+			expect_equal(read_names(), ["command"])
+		)
+	)
+
+	define("unsatisfiable raw commands", func():
+		check_invalid_raw("missing size", "\rcommand\n")
+		check_invalid_raw("non-numeric size", "\rcommand foo\n")
+		check_invalid_raw("blank line", "\r\n")
+		check_invalid_raw("negative size", "\rcommand -4\n")
+		check_invalid_raw("size over max_size", "\rcommand 100000\n")
+		check_invalid_raw("size just over max_size", "\rcommand 16385\n")
+		# The terminating newline wouldn't fit in the buffer
+		check_invalid_raw("size at max_size", "\rcommand 16384\n")
+
+		test("should keep parsing after rejecting a raw command", func():
+			reader.ingest_text("\r\n")
+			expect_null(reader.read(), "Command was read!")
+			expect_not_equal(reader.last_error, OK, "No error was reported!")
+
+			reader.ingest_text("command foo\n")
+			expect_equal(read_names(), ["command"])
+		)
+
+		test("should accept raw command sized up to max_size", func():
+			reader.max_size = 16
+
+			# 15 bytes of data plus the terminating newline exactly fill the buffer
+			reader.ingest_text("\rcommand 15\n")
+			expect_null(reader.read(), "Command was read without data!")
+
+			reader.ingest_text("012345678901234\n")
+			var command := reader.read()
+
+			expect_not_null(command)
+			expect_equal(command.raw, "012345678901234".to_utf8_buffer())
+		)
+	)
+
+	define("buffer overflow", func():
+		test("should reject data over max_size", func():
+			reader.max_size = 8
+
+			expect_not_equal(reader.ingest_text("command foobar\n"), OK)
+		)
+
+		test("should resume parsing after discarding the buffer", func():
+			reader.max_size = 16
+
+			reader.ingest_text("012345678901")
+			expect_null(reader.read(), "Command was read!")
+
+			expect_not_equal(reader.ingest_text("01234"), OK, "No overflow!")
+
+			reader.ingest_text("command\n")
+			expect_equal(read_names(), ["command"])
+		)
+
+		test("should reset quote state after discarding the buffer", func():
+			reader.max_size = 16
+
+			reader.ingest_text("\"")
+			expect_null(reader.read(), "Command was read!")
+
+			expect_not_equal(reader.ingest_text("0123456789012345"), OK, "No overflow!")
+
+			reader.ingest_text("command foo\n")
+			expect_equal(read_names(), ["command"])
+		)
+
+		test("should drop queued raw command after discarding the buffer", func():
+			reader.max_size = 16
+
+			reader.ingest_text("\rcommand 8\n")
+			expect_null(reader.read(), "Command was read without data!")
+
+			expect_not_equal(reader.ingest_text("0".repeat(17)), OK, "No overflow!")
+
+			reader.ingest_text("command foo\n")
+			expect_equal(read_names(), ["command"])
+		)
+	)
+
+func check_invalid_raw(name: String, input: String) -> void:
+	test("should reject raw command with " + name, func():
+		reader.ingest_text(input)
+
+		expect_null(reader.read(), "Command was read!")
+		expect_not_equal(reader.last_error, OK, "No error was reported!")
+	)
+
+# Read all the commands available and return their names
+func read_names() -> Array:
+	var names := []
+	while true:
+		var command := reader.read()
+		if not command: break
+		names.append(command.name)
+
+	return names