Re: [PATCH spice-streaming-agent v5 2/2] Implement handling of error messages from the server

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03/04/2018 12:38 PM, Uri Lublin wrote:
On 02/28/2018 02:53 PM, Lukáš Hrázký wrote:
Log error messages from the server to syslog (not aborting the agent).
Limits the messages to 1023 bytes, error messages from the server are
not expected to be longer. In the unlikely case the message is longer,
log the first 1023 bytes and throw an exception (because we don't read
out the rest of the message, causing desynchronization between the
server and the streaming agent).

Signed-off-by: Lukáš Hrázký <lhrazky@xxxxxxxxxx>
---
  src/spice-streaming-agent.cpp | 26 +++++++++++++++++++++++---
  1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
index ee57920..954d1b3 100644
--- a/src/spice-streaming-agent.cpp
+++ b/src/spice-streaming-agent.cpp
@@ -137,10 +137,30 @@ static void handle_stream_capabilities(uint32_t len)
      }
  }
-static void handle_stream_error(uint32_t len)
+static void handle_stream_error(size_t len)
  {
-    // TODO read message and use it
-    throw std::runtime_error("got an error message from server");
+    if (len < sizeof(StreamMsgNotifyError)) {
+        throw std::runtime_error("Received NotifyError message size " + std::to_string(len) +
+                                 " is too small (smaller than " +
+ std::to_string(sizeof(StreamMsgNotifyError)) + ")");
+    }
+
+    struct : StreamMsgNotifyError {
+        uint8_t msg[1024];
+    } msg;
+
+    size_t len_to_read = std::min(len, sizeof(msg) - 1);
+
+    read_all(&msg, len_to_read);
+    msg.msg[len_to_read - sizeof(StreamMsgNotifyError)] = '\0';

That looks wrong. It should be:
        msg.msg[len_to_read] = '\0';
Otherwise, the end of the message (last 4 bytes) is cut out.


After re-reading it, I realize there are two meanings to msg.msg:
one from the definition above and one inherited.

After re-re-reading it, it seems the code is correct but confusing.
The local variable msg.msg and the inherited msg.msg point to the
same location in memory (after StreamMsgNotifyError.error_code).

Uri.


Uri.
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]