Merge pull request nanopb#230 from Tobba/nonempty-substring

Fix closing a non-empty substream resulting in an incorrect stream state

docs/migration.rst

@@ -11,6 +11,19 @@ are included, in order to make it easier to find this document.
 
 .. contents ::
 
+Nanopb-0.3.8 (2017-xx-xx)
+=========================
+Fully drain substreams before closing
+
+**Rationale:** If the substream functions were called directly and the caller
+did not completely empty the substring before closing it, the parent stream
+would be put into an incorrect state.
+
+**Changes:** *pb_close_string_substream* can now error and returns a boolean.
+
+**Required actions:** Add error checking onto any call to
+*pb_close_string_substream*.
+
 Nanopb-0.3.5 (2016-02-13)
 =========================
 

pb_decode.c

@@ -333,13 +333,19 @@ bool checkreturn pb_make_string_substream(pb_istream_t *stream, pb_istream_t *su
     return true;
 }
 
-void pb_close_string_substream(pb_istream_t *stream, pb_istream_t *substream)
+bool checkreturn pb_close_string_substream(pb_istream_t *stream, pb_istream_t *substream)
 {
+    if (substream->bytes_left) {
+        if (!pb_read(substream, NULL, substream->bytes_left))
+            return false;
+    }
+
     stream->state = substream->state;
 
 #ifndef PB_NO_ERRMSG
     stream->errmsg = substream->errmsg;
 #endif
+    return true;
 }
 
 /*************************
@@ -385,11 +391,12 @@ static bool checkreturn decode_static_field(pb_istream_t *stream, pb_wire_type_t
                     }
                     (*size)++;
                 }
-                pb_close_string_substream(stream, &substream);
-
+
                 if (substream.bytes_left != 0)
                     PB_RETURN_ERROR(stream, "array overflow");
-
+                if (!pb_close_string_substream(stream, &substream))
+                    return false;
+
                 return status;
             }
             else
@@ -569,7 +576,8 @@ static bool checkreturn decode_pointer_field(pb_istream_t *stream, pb_wire_type_
 
                     (*size)++;
                 }
-                pb_close_string_substream(stream, &substream);
+                if (!pb_close_string_substream(stream, &substream))
+                    return false;
 
                 return status;
             }
@@ -623,7 +631,9 @@ static bool checkreturn decode_callback_field(pb_istream_t *stream, pb_wire_type
                 PB_RETURN_ERROR(stream, "callback failed");
         } while (substream.bytes_left);
 
-        pb_close_string_substream(stream, &substream);
+        if (!pb_close_string_substream(stream, &substream))
+            return false;
+
         return true;
     }
     else
@@ -964,7 +974,9 @@ bool pb_decode_delimited(pb_istream_t *stream, const pb_field_t fields[], void *
         return false;
 
     status = pb_decode(&substream, fields, dest_struct);
-    pb_close_string_substream(stream, &substream);
+
+    if (!pb_close_string_substream(stream, &substream))
+        return false;
     return status;
 }
 
@@ -1343,6 +1355,7 @@ static bool checkreturn pb_dec_submessage(pb_istream_t *stream, const pb_field_t
     else
         status = pb_decode_noinit(&substream, submsg_fields, dest);
 
-    pb_close_string_substream(stream, &substream);
+    if (!pb_close_string_substream(stream, &substream))
+        return false;
     return status;
 }

pb_decode.h

@@ -144,7 +144,7 @@ bool pb_decode_fixed64(pb_istream_t *stream, void *dest);
 
 /* Make a limited-length substream for reading a PB_WT_STRING field. */
 bool pb_make_string_substream(pb_istream_t *stream, pb_istream_t *substream);
-void pb_close_string_substream(pb_istream_t *stream, pb_istream_t *substream);
+bool pb_close_string_substream(pb_istream_t *stream, pb_istream_t *substream);
 
 #ifdef __cplusplus
 } /* extern "C" */