When we're dissecting the beginning of a fragmented packet that we

haven't reassembled, we're probably moving sequentially through the
packet, which means that we'll run past the end of the fragment rather
than past the end of what would have been the reassembled packet had we
reassembled it.

I.e., there's little reason to care whether we're past the end of the
fragment but not past the end of the packet, or whether we're past the
end of the packet; in either case, we're past the end of the fragment,
and if somebody wants to know whether the packet is malformed by
stopping short of certain fields, they should enable reassembly.

So we get rid of the explicit fragment length in tvbuffs and, instead,
have a "this is a fragment" flag; if that flag is set, we throw
FragmentBoundsError rather than ReportedBoundsError if we run past the
end of the reported data.

(This also means we could flag the tvbuff even if we don't know how
large the reassembled packet will be, e.g. when doing IP reassembly.)

Replace tvb_new_subset_length_fragment() with tvb_new_subset_length()
and a new "set the "this is a fragment flag"" routine.

svn path=/trunk/; revision=48940

4 files changed, 84 insertions, 134 deletions

diff --git a/epan/dissectors/packet-clnp.c b/epan/dissectors/packet-clnp.c
index 99182eed27..517a65e894 100644
--- a/epan/dissectors/packet-clnp.c
+++ b/epan/dissectors/packet-clnp.c
@@ -536,10 +536,14 @@ dissect_clnp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
     } else {
       /* First segment, or not segmented.  Dissect what we have here. */
 
-      /* Get a tvbuff for the payload. */
-      next_tvb = tvb_new_subset_length_fragment(tvb, offset,
-                                                segment_length - cnf_hdr_len,
-                                                total_length - cnf_hdr_len);
+      /* Get a tvbuff for the payload.  Set its length to the segment
+         length, and flag it as a fragment, so going past the end
+         reports FragmentBoundsError, i.e. "there's data missing
+         because this isn't reassembled", not ReportedBoundsError,
+         i.e. "the dissector ran past the end of the packet, so the
+         packet must not have been constructed properly". */
+      next_tvb = tvb_new_subset_length(tvb, offset, segment_length - cnf_hdr_len);
+      tvb_set_fragment(next_tvb);
 
       /*
        * If this is the first segment, but not the only segment,
diff --git a/epan/tvbuff-int.h b/epan/tvbuff-int.h
index 33057bdd87..6f7e9818a0 100644
--- a/epan/tvbuff-int.h
+++ b/epan/tvbuff-int.h
@@ -48,6 +48,11 @@ typedef struct {
 
 } tvb_comp_t;
 
+/*
+ * Tvbuff flags.
+ */
+#define TVBUFF_FRAGMENT		0x00000001	/* this is a fragment */
+
 struct tvbuff {
 	/* Doubly linked list pointers */
 	tvbuff_t                *next;
@@ -56,6 +61,7 @@ struct tvbuff {
 	/* Record-keeping */
 	tvbuff_type		type;
 	gboolean		initialized;
+	guint			flags;
 	struct tvbuff		*ds_tvb;  /**< data source top-level tvbuff */
 
 	/** TVBUFF_SUBSET and TVBUFF_COMPOSITE keep track
@@ -79,12 +85,6 @@ struct tvbuff {
 	/** Reported length. */
 	guint			reported_length;
 
-	/* If this tvbuff represents the first fragment of a larger packet
-	 * that was not reassembled, this is the length of the fragment
-	 * ("reported_length" will be the length of the full packet).
-	 */
-	guint			fragment_length;
-
 	/* Offset from beginning of first TVBUFF_REAL. */
 	gint			raw_offset;
 
diff --git a/epan/tvbuff.c b/epan/tvbuff.c
index 1dff1b26d0..0cb3d55346 100644
--- a/epan/tvbuff.c
+++ b/epan/tvbuff.c
@@ -65,7 +65,7 @@ _tvb_get_bits64(tvbuff_t *tvb, guint bit_offset, const gint total_no_of_bits);
 static tvbuff_t *
 tvb_new(const tvbuff_type type)
 {
-	tvbuff_t *tvb;
+	tvbuff_t	*tvb;
 	tvb_backing_t	*backing;
 	tvb_comp_t	*composite;
 
@@ -75,9 +75,9 @@ tvb_new(const tvbuff_type type)
 	tvb->next	     = NULL;
 	tvb->type	     = type;
 	tvb->initialized     = FALSE;
+	tvb->flags	     = 0;
 	tvb->length	     = 0;
 	tvb->reported_length = 0;
-	tvb->fragment_length = 0;
 	tvb->free_cb	     = NULL;
 	tvb->real_data	     = NULL;
 	tvb->raw_offset	     = -1;
@@ -220,7 +220,6 @@ tvb_new_real_data(const guint8* data, const guint length, const gint reported_le
 	tvb->real_data       = data;
 	tvb->length          = length;
 	tvb->reported_length = reported_length;
-	tvb->fragment_length = reported_length;
 	tvb->initialized     = TRUE;
 
 	/*
@@ -268,13 +267,11 @@ compute_offset_length(const tvbuff_t *tvb,
 		/* Positive offset - relative to the beginning of the packet. */
 		if ((guint) offset > tvb->reported_length) {
 			if (exception) {
-				*exception = ReportedBoundsError;
-			}
-			return FALSE;
-		}
-		else if ((guint) offset > tvb->fragment_length) {
-			if (exception) {
-				*exception = FragmentBoundsError;
+				if (tvb->flags & TVBUFF_FRAGMENT) {
+					*exception = FragmentBoundsError;
+				} else {
+					*exception = ReportedBoundsError;
+				}
 			}
 			return FALSE;
 		}
@@ -292,13 +289,11 @@ compute_offset_length(const tvbuff_t *tvb,
 		/* Negative offset - relative to the end of the packet. */
 		if ((guint) -offset > tvb->reported_length) {
 			if (exception) {
-				*exception = ReportedBoundsError;
-			}
-			return FALSE;
-		}
-		else if ((guint) -offset > tvb->fragment_length) {
-			if (exception) {
-				*exception = FragmentBoundsError;
+				if (tvb->flags & TVBUFF_FRAGMENT) {
+					*exception = FragmentBoundsError;
+				} else {
+					*exception = ReportedBoundsError;
+				}
 			}
 			return FALSE;
 		}
@@ -368,14 +363,15 @@ check_offset_length_no_exception(const tvbuff_t *tvb,
 	}
 	else {
 		if (exception) {
-			if (end_offset <= tvb->fragment_length) {
+			if (end_offset <= tvb->reported_length) {
 				*exception = BoundsError;
 			}
-			else if (end_offset <= tvb->reported_length) {
-				*exception = FragmentBoundsError;
-			}
 			else {
-				*exception = ReportedBoundsError;
+				if (tvb->flags & TVBUFF_FRAGMENT) {
+					*exception = FragmentBoundsError;
+				} else {
+					*exception = ReportedBoundsError;
+				}
 			}
 		}
 
@@ -401,9 +397,8 @@ check_offset_length(const tvbuff_t *tvb,
 }
 
 static tvbuff_t *
-tvb_new_with_subset(tvbuff_t *backing, const gint fragment_length,
-    const gint reported_length, const guint subset_tvb_offset,
-    const guint subset_tvb_length)
+tvb_new_with_subset(tvbuff_t *backing, const gint reported_length,
+    const guint subset_tvb_offset, const guint subset_tvb_length)
 {
 	tvbuff_t *tvb = tvb_new(TVBUFF_SUBSET);
 
@@ -414,14 +409,11 @@ tvb_new_with_subset(tvbuff_t *backing, const gint fragment_length,
 	tvb->length		     = tvb->tvbuffs.subset.length;
 
 	if (reported_length == -1) {
-		tvb->fragment_length = backing->fragment_length - tvb->tvbuffs.subset.offset;
 		tvb->reported_length = backing->reported_length - tvb->tvbuffs.subset.offset;
 	}
 	else {
-		tvb->fragment_length = fragment_length;
 		tvb->reported_length = reported_length;
 	}
-
 	tvb->initialized	     = TRUE;
 	add_to_chain(backing, tvb);
 
@@ -449,7 +441,7 @@ tvb_new_subset(tvbuff_t *backing, const gint backing_offset, const gint backing_
 			    &subset_tvb_offset,
 			    &subset_tvb_length);
 
-	tvb = tvb_new_with_subset(backing, reported_length, reported_length,
+	tvb = tvb_new_with_subset(backing, reported_length,
 	    subset_tvb_offset, subset_tvb_length);
 
 	/*
@@ -485,46 +477,7 @@ tvb_new_subset_length(tvbuff_t *backing, const gint backing_offset, const gint b
 			    &subset_tvb_offset,
 			    &subset_tvb_length);
 
-	tvb = tvb_new_with_subset(backing, backing_length, backing_length,
-	    subset_tvb_offset, subset_tvb_length);
-
-	/*
-	 * The top-level data source of this tvbuff is the top-level
-	 * data source of its parent.
-	 */
-	tvb->ds_tvb = backing->ds_tvb;
-
-	return tvb;
-}
-
-tvbuff_t *
-tvb_new_subset_length_fragment(tvbuff_t *backing, const gint backing_offset,
-    const gint fragment_length, const gint reported_length)
-{
-	gint	  captured_length;
-	tvbuff_t *tvb;
-	guint	  subset_tvb_offset;
-	guint	  subset_tvb_length;
-
-	DISSECTOR_ASSERT(backing && backing->initialized);
-
-	THROW_ON(fragment_length < 0, ReportedBoundsError);
-	THROW_ON(reported_length < 0, ReportedBoundsError);
-	THROW_ON(reported_length < fragment_length, ReportedBoundsError);
-
-	/*
-	 * Give the next dissector only captured_length bytes.
-	 */
-	captured_length = tvb_length_remaining(backing, backing_offset);
-	THROW_ON(captured_length < 0, BoundsError);
-	if (captured_length > fragment_length)
-		captured_length = fragment_length;
-
-	check_offset_length(backing, backing_offset, captured_length,
-			    &subset_tvb_offset,
-			    &subset_tvb_length);
-
-	tvb = tvb_new_with_subset(backing, fragment_length, reported_length,
+	tvb = tvb_new_with_subset(backing, backing_length,
 	    subset_tvb_offset, subset_tvb_length);
 
 	/*
@@ -547,8 +500,8 @@ tvb_new_subset_remaining(tvbuff_t *backing, const gint backing_offset)
 			    &subset_tvb_offset,
 			    &subset_tvb_length);
 
-	tvb = tvb_new_with_subset(backing, -1 /* fragment_length */,
-	    -1 /* reported_length */, subset_tvb_offset, subset_tvb_length);
+	tvb = tvb_new_with_subset(backing, -1 /* reported_length */,
+	    subset_tvb_offset, subset_tvb_length);
 
 	/*
 	 * The top-level data source of this tvbuff is the top-level
@@ -687,7 +640,6 @@ tvb_composite_finalize(tvbuff_t *tvb)
 	DISSECTOR_ASSERT(tvb && !tvb->initialized);
 	DISSECTOR_ASSERT(tvb->type == TVBUFF_COMPOSITE);
 	DISSECTOR_ASSERT(tvb->length == 0);
-	DISSECTOR_ASSERT(tvb->fragment_length == 0);
 	DISSECTOR_ASSERT(tvb->reported_length == 0);
 
 	composite   = &tvb->tvbuffs.composite;
@@ -707,7 +659,6 @@ tvb_composite_finalize(tvbuff_t *tvb)
 		member_tvb = (tvbuff_t *)slist->data;
 		composite->start_offsets[i] = tvb->length;
 		tvb->length += member_tvb->length;
-		tvb->fragment_length += member_tvb->fragment_length;
 		tvb->reported_length += member_tvb->reported_length;
 		composite->end_offsets[i] = tvb->length - 1;
 		i++;
@@ -758,11 +709,13 @@ tvb_ensure_length_remaining(const tvbuff_t *tvb, const gint offset)
 		 * There aren't any bytes available, so throw the appropriate
 		 * exception.
 		 */
-		if (abs_offset >= tvb->reported_length)
-			THROW(ReportedBoundsError);
-		else if (abs_offset >= tvb->fragment_length)
-			THROW(FragmentBoundsError);
-		else
+		if (abs_offset >= tvb->reported_length) {
+			if (tvb->flags & TVBUFF_FRAGMENT) {
+				THROW(FragmentBoundsError);
+			} else {
+				THROW(ReportedBoundsError);
+			}
+		} else
 			THROW(BoundsError);
 	}
 	return abs_length;
@@ -862,10 +815,8 @@ tvb_reported_length_remaining(const tvbuff_t *tvb, const gint offset)
 /* Set the reported length of a tvbuff to a given value; used for protocols
  * whose headers contain an explicit length and where the calling
  * dissector's payload may include padding as well as the packet for
- * this protocol, or where a packet may be a fragment of a larger packet
- * and the length of the larger packet is known.
- *
- * Also adjusts the data length and fragment length. */
+ * this protocol.
+ * Also adjusts the data length. */
 void
 tvb_set_reported_length(tvbuff_t *tvb, const guint reported_length)
 {
@@ -877,8 +828,6 @@ tvb_set_reported_length(tvbuff_t *tvb, const guint reported_length)
 	tvb->reported_length = reported_length;
 	if (reported_length < tvb->length)
 		tvb->length = reported_length;
-	if (reported_length < tvb->fragment_length)
-		tvb->fragment_length = reported_length;
 }
 
 
@@ -1043,10 +992,12 @@ fast_ensure_contiguous(tvbuff_t *tvb, const gint offset, const guint length)
 	}
 
 	if (end_offset > tvb->reported_length) {
-		THROW(ReportedBoundsError);
-	}
-	if (end_offset > tvb->fragment_length) {
-		THROW(FragmentBoundsError);
+		if (tvb->flags & TVBUFF_FRAGMENT) {
+			THROW(FragmentBoundsError);
+		} else {
+			THROW(ReportedBoundsError);
+		}
+		/* not reached */
 	}
 	THROW(BoundsError);
 	/* not reached */
@@ -2081,25 +2032,20 @@ tvb_strsize(tvbuff_t *tvb, const gint offset)
 		 * OK, we hit the end of the tvbuff, so we should throw
 		 * an exception.
 		 *
-		 * Did we hit the end of the captured data, the end of
-		 * the fragment data, or the end of the actual data?
-		 *
-		 * If there's less captured data than fragment data,
-		 * we presumably hit the end of the captured data.
-		 *
-		 * Otherwise, if there's less fragment data than actual
-		 * data, we presumably hit the end of the fragment data.
-		 *
-		 * Otherwise we hit the end of the actual data.
+		 * Did we hit the end of the captured data, or the end
+		 * of the actual data?	If there's less captured data
+		 * than actual data, we presumably hit the end of the
+		 * captured data, otherwise we hit the end of the actual
+		 * data.
 		 */
-		if (tvb->length < tvb->fragment_length) {
+		if (tvb->length < tvb->reported_length) {
 			THROW(BoundsError);
-		}
-		else if (tvb->fragment_length < tvb->reported_length) {
-			THROW(FragmentBoundsError);
-		}
-		else {
-			THROW(ReportedBoundsError);
+		} else {
+			if (tvb->flags & TVBUFF_FRAGMENT) {
+				THROW(FragmentBoundsError);
+			} else {
+				THROW(ReportedBoundsError);
+			}
 		}
 	}
 	return (nul_offset - abs_offset) + 1;
@@ -3706,6 +3652,12 @@ tvb_raw_offset(tvbuff_t *tvb)
 	return ((tvb->raw_offset==-1)?(tvb->raw_offset = tvb_offset_from_real_beginning(tvb)):tvb->raw_offset);
 }
 
+void
+tvb_set_fragment(tvbuff_t *tvb)
+{
+	tvb->flags |= TVBUFF_FRAGMENT;
+}
+
 struct tvbuff *
 tvb_get_ds_tvb(tvbuff_t *tvb)
 {
diff --git a/epan/tvbuff.h b/epan/tvbuff.h
index 21abf5ecf7..8950feb68b 100644
--- a/epan/tvbuff.h
+++ b/epan/tvbuff.h
@@ -81,10 +81,10 @@ typedef struct tvbuff tvbuff_t;
  * A dissector:
  *  - Can chain new tvbs (subset, real, composite) to the
  *    tvb handed to the dissector using tvb_new_subset(),
- *    tvb_new_subset_remaining(), tvb_new_child_real_data(),
- *    tvb_set_child_real_data_tvbuff(), tvb_composite_finalize(), and
- *    tvb_child_uncompress(). (Composite tvbs should reference
- *    only tvbs which are already part of the chain).
+ *    tvb_new_subset_length(), tvb_new_subset_remaining(),
+ *    tvb_new_child_real_data(), tvb_set_child_real_data_tvbuff(),
+ *    tvb_composite_finalize(), and tvb_child_uncompress(). (Composite
+ *    tvbs should reference only tvbs which are already part of the chain).
  *  - Must not save for later use (e.g., when dissecting another frame) a
  *    pointer to a tvb handed to the dissector; (A higher level function
  *    may very well free the chain thus leaving a dangling pointer).
@@ -181,24 +181,14 @@ WS_DLL_PUBLIC tvbuff_t* tvb_new_real_data(const guint8* data, const guint length
 WS_DLL_PUBLIC tvbuff_t* tvb_new_subset(tvbuff_t* backing,
 		const gint backing_offset, const gint backing_length, const gint reported_length);
 
-/*
-* Similar to tvb_new_subset() but with captured length calculated
+/**
+ * Similar to tvb_new_subset() but with captured length calculated
  * to fit within the existing captured length and the specified
- * backing length (which is used as both the fragment and reported
- * length).
+ * backing length (which is used as the reported length).
  * Can throw ReportedBoundsError. */
 WS_DLL_PUBLIC tvbuff_t* tvb_new_subset_length(tvbuff_t *backing,
 		const gint backing_offset, const gint backing_length);
 
-/** Similar to tvb_new_subset() but with fragment and reported length
- * set as specified and captured length calculated to fit within
- * the existing captured length and the specified fragment and
- * reported lengths.
- * Can throw ReportedBoundsError. */
-extern tvbuff_t* tvb_new_subset_length_fragment(tvbuff_t *backing,
-		const gint backing_offset, const gint fragment_length,
-		const gint reported_length);
-
 /** Similar to tvb_new_subset() but with backing_length and reported_length set to -1.
  * Can throw ReportedBoundsError. */
 WS_DLL_PUBLIC tvbuff_t* tvb_new_subset_remaining(tvbuff_t* backing,
@@ -267,6 +257,12 @@ WS_DLL_PUBLIC guint tvb_offset_from_real_beginning(const tvbuff_t *tvb);
 /* Returns the offset from the first byte of real data. */
 WS_DLL_PUBLIC gint tvb_raw_offset(tvbuff_t *tvb);
 
+/** Set the "this is a fragment" flag. */
+WS_DLL_PUBLIC void tvb_set_fragment(tvbuff_t *tvb);
+
+WS_DLL_PUBLIC struct tvbuff *tvb_get_ds_tvb(tvbuff_t *tvb);
+
+
 /************** START OF ACCESSORS ****************/
 /* All accessors will throw an exception if appropriate */
 
@@ -678,8 +674,6 @@ typedef struct dgt_set_t
 dgt_set_t;
 
 WS_DLL_PUBLIC const gchar *tvb_bcd_dig_to_ep_str(tvbuff_t *tvb, const gint offset, const gint len, dgt_set_t *dgt, gboolean skip_first);
-WS_DLL_PUBLIC
-struct tvbuff *tvb_get_ds_tvb(tvbuff_t *tvb);
 
 /** Locate a sub-tvbuff within another tvbuff, starting at position
  * 'haystack_offset'. Returns the index of the beginning of 'needle' within