[zeromq-dev] [PATCH] decoder asserts

Dhammika Pathirana dhammika at gmail.com
Thu Oct 21 08:17:02 CEST 2010


Patch to handle decoding errors.

>From 0a179e4c16e390e091629ab83f08b813ae5ad4ed Mon Sep 17 00:00:00 2001
From: dhammika <dhammika at gmail.com>
Date: Wed, 20 Oct 2010 19:27:00 -0700
Subject: [PATCH] handle decoding malformed messages


Signed-off-by: dhammika <dhammika at gmail.com>
---
 src/decoder.cpp    |   26 ++++++++++++--------------
 src/decoder.hpp    |   25 +++++++++++++++++++++----
 src/zmq_engine.cpp |   24 ++++++++++++++----------
 3 files changed, 47 insertions(+), 28 deletions(-)

diff --git a/src/decoder.cpp b/src/decoder.cpp
index 131ee24..49febc0 100644
--- a/src/decoder.cpp
+++ b/src/decoder.cpp
@@ -54,16 +54,16 @@ bool zmq::decoder_t::one_byte_size_ready ()
         next_step (tmpbuf, 8, &decoder_t::eight_byte_size_ready);
     else {

-        //  TODO:  Handle over-sized message decently.
-
         //  There has to be at least one byte (the flags) in the message).
-        zmq_assert (*tmpbuf > 0);
-
         //  in_progress is initialised at this point so in theory we should
         //  close it before calling zmq_msg_init_size, however, it's a 0-byte
         //  message and thus we can treat it as uninitialised...
-        int rc = zmq_msg_init_size (&in_progress, *tmpbuf - 1);
-        errno_assert (rc == 0);
+        if (!*tmpbuf ||
+            zmq_msg_init_size (&in_progress, *tmpbuf - 1)) {
+            decoding_error ();
+            return false;
+        }
+
         next_step (tmpbuf, 1, &decoder_t::flags_ready);
     }
     return true;
@@ -75,19 +75,17 @@ bool zmq::decoder_t::eight_byte_size_ready ()
     //  read the message data into it.
     size_t size = (size_t) get_uint64 (tmpbuf);

-    //  TODO:  Handle over-sized message decently.
-
     //  There has to be at least one byte (the flags) in the message).
-    zmq_assert (size > 0);
-
-
     //  in_progress is initialised at this point so in theory we should
     //  close it before calling zmq_msg_init_size, however, it's a 0-byte
     //  message and thus we can treat it as uninitialised...
-    int rc = zmq_msg_init_size (&in_progress, size - 1);
-    errno_assert (rc == 0);
-    next_step (tmpbuf, 1, &decoder_t::flags_ready);
+    if (!size ||
+        zmq_msg_init_size (&in_progress, size - 1)) {
+        decoding_error ();
+        return false;
+    }

+    next_step (tmpbuf, 1, &decoder_t::flags_ready);
     return true;
 }

diff --git a/src/decoder.hpp b/src/decoder.hpp
index 87982a0..bf46cd6 100644
--- a/src/decoder.hpp
+++ b/src/decoder.hpp
@@ -98,9 +98,14 @@ namespace zmq
                 read_pos += size_;
                 to_read -= size_;

-                while (!to_read)
-                    if (!(static_cast <T*> (this)->*next) ())
+                while (!to_read) {
+                    if (!(static_cast <T*> (this)->*next) ()) {
+                        if (unlikely (!(static_cast <T*> (this)->next)))
+                            return (size_t) -1;
+
                         return size_;
+                    }
+                }
                 return size_;
             }

@@ -109,9 +114,14 @@ namespace zmq

                 //  Try to get more space in the message to fill in.
                 //  If none is available, return.
-                while (!to_read)
-                    if (!(static_cast <T*> (this)->*next) ())
+                while (!to_read) {
+                    if (!(static_cast <T*> (this)->*next) ()) {
+                        if (unlikely (!(static_cast <T*> (this)->next)))
+                            return (size_t) -1;
+
                         return pos;
+                    }
+                }

                 //  If there are no more data in the buffer, return.
                 if (pos == size_)
@@ -142,6 +152,13 @@ namespace zmq
             next = next_;
         }

+        //  This function should be called from the derived class to
+        //  abort decoder state machine.
+        inline void decoding_error()
+        {
+            next = NULL;
+        }
+
     private:

         unsigned char *read_pos;
diff --git a/src/zmq_engine.cpp b/src/zmq_engine.cpp
index 815697c..bde0998 100644
--- a/src/zmq_engine.cpp
+++ b/src/zmq_engine.cpp
@@ -119,19 +119,23 @@ void zmq::zmq_engine_t::in_event ()
     //  Push the data to the decoder.
     size_t processed = decoder.process_buffer (inpos, insize);

-    //  Stop polling for input if we got stuck.
-    if (processed < insize) {
+    if (unlikely (processed == (size_t) -1)) {
+        disconnection = true;
+    } else {
+        //  Stop polling for input if we got stuck.
+        if (processed < insize) {
+
+            //  This may happen if queue limits are in effect or when
+            //  init object reads all required information from the socket
+            //  and rejects to read more data.
+            reset_pollin (handle);
+        }

-        //  This may happen if queue limits are in effect or when
-        //  init object reads all required information from the socket
-        //  and rejects to read more data.
-        reset_pollin (handle);
+        //  Adjust the buffer.
+        inpos += processed;
+        insize -= processed;
     }

-    //  Adjust the buffer.
-    inpos += processed;
-    insize -= processed;
-
     //  Flush all messages the decoder may have produced.
     inout->flush ();

-- 
1.7.0.4



More information about the zeromq-dev mailing list