Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/guacamole-server/pull/183#discussion_r217132320
--- Diff: src/libguac/parser.c ---
@@ -222,7 +222,11 @@ int guac_parser_read(guac_parser* parser, guac_socket*
socket, int usec_timeout)
retval = guac_socket_select(socket, usec_timeout);
if (retval <= 0)
return -1;
-
+
+ /* Reset pointers if instruction buf len is less than max
instruction len */
+ if (buffer_end - unparsed_end < GUAC_INSTRUCTION_MAX_LENGTH)
+ unparsed_end = unparsed_start = parser->__instructionbuf;
--- End diff --
Well, the good news is that your servlet is absolved. There can be no doubt
that the protocol data is indeed reaching guacd correctly and is not being
clobbered by shared use of a common socket as I originally suspected. [Your
provided
`write_log.txt`](https://issues.apache.org/jira/secure/attachment/12939404/write-log.txt)
clearly demonstrates this:
```
...
--- 23 bytes:
00000000: 35 2E 6D 6F 75 73 65 2C 33 2E 35 34 31 2C 32 2E 5.mouse,3.541,2.
00000010: 32 36 2C 31 2E 31 3B 26,1.1;
--- 23 bytes:
00000000: 35 2E 6D 6F 75 73 65 2C 33 2E 35 34 32 2C 32 2E 5.mouse,3.542,2.
00000010: 32 36 2C 31 2E 31 3B 26,1.1;
```
The bad news is that these changes are definitely incorrect, and they do
indeed break the parser. The following are the blocks ultimately read by
simple, sequential calls to `guac_socket_read()` within the parser according to
[your provided
`read.txt`](https://issues.apache.org/jira/secure/attachment/12939405/read-log.txt):
```
...
--- 20 bytes:
00000000: 35 2E 6D 6F 75 73 65 2C 33 2E 35 34 31 2C 32 2E 5.mouse,3.541,2.
00000010: 32 36 2C 31 26,1
--- 23 bytes:
00000000: 35 2E 6D 6F 75 73 65 2C 33 2E 35 34 32 2C 32 2E 5.mouse,3.542,2.
00000010: 32 36 2C 31 2E 31 3B 26,1.1;
```
Any changes which allow the parser to accept
"5.mouse,3.541,2.26,15.mouse,3.542,2.26,1.1;" are incorrect, as that should
result in a parse error. It is invalid protocol data.
The issue is not that the parser is behaving incorrectly - it is correctly
bailing out with a parse error when given invalid data. The issue is that bytes
are somehow vanishing from the stream between otherwise normal calls to
`read()`.
---