Add tvb_get and proto_tree_add for string-encoded byte arrays

This commit adds tvb_get_string_bytes and proto_tree_add_bytes_item routines for
getting GByteArrays fields from the tvb when they are encoded in ASCII hex string form.

The proto_tree_add_bytes_item routine is also usable for normal
binary encoded byte arrays, and has the advantage of retrieving
the array values even if there's no proto tree.

It also exposes the routines to Lua, both so that a Lua script can take
advantage of this, but also so I can write a testsuite to test the functions.

Change-Id: I112a038653df6482a5d0ebe7c95708f207319e20
Reviewed-on: https://code.wireshark.org/review/1158
Reviewed-by: Hadriel Kaplan <hadrielk@yahoo.com>
Reviewed-by: Anders Broman <a.broman58@gmail.com>

9 files changed, 510 insertions, 12 deletions

diff --git a/epan/proto.c b/epan/proto.c
index 2707f056ec..ac73a2604b 100644
--- a/epan/proto.c
+++ b/epan/proto.c
@@ -87,8 +87,9 @@ struct ptvcursor {
  @param tree the tree to append this item to
  @param hfindex field index
  @param hfinfo header_field
+ @param free_block a code block to call to free resources if this returns
  @return the header field matching 'hfinfo' */
-#define TRY_TO_FAKE_THIS_ITEM(tree, hfindex, hfinfo) \
+#define TRY_TO_FAKE_THIS_ITEM_OR_FREE(tree, hfindex, hfinfo, free_block) \
 	/* If this item is not referenced we dont have to do much work	\
 	   at all but we should still return a node so that field items	\
 	   below this node (think proto_item_add_subtree()) will still	\
@@ -100,10 +101,13 @@ struct ptvcursor {
 	   We fake FT_PROTOCOL unless some clients have requested us	\
 	   not to do so. \
 	*/								\
-	if (!tree)							\
+	if (!tree) {							\
+		free_block;						\
 		return NULL;						\
+	}								\
 	PTREE_DATA(tree)->count++;					\
 	if (PTREE_DATA(tree)->count > MAX_TREE_ITEMS) {			\
+		free_block;						\
 		if (getenv("WIRESHARK_ABORT_ON_TOO_MANY_ITEMS") != NULL) \
 			g_error("More than %d items in the tree -- possible infinite loop", MAX_TREE_ITEMS); \
 		/* Let the exception handler add items to the tree */	\
@@ -117,6 +121,7 @@ struct ptvcursor {
 			if ((hfinfo->ref_type != HF_REF_TYPE_DIRECT)	\
 			    && (hfinfo->type != FT_PROTOCOL ||		\
 				PTREE_DATA(tree)->fake_protocols)) {	\
+				free_block;				\
 				/* just return tree back to the caller */\
 				return tree;				\
 			}						\
@@ -124,6 +129,15 @@ struct ptvcursor {
 	}
 
 /** See inlined comments.
+ @param tree the tree to append this item to
+ @param hfindex field index
+ @param hfinfo header_field
+ @return the header field matching 'hfinfo' */
+#define TRY_TO_FAKE_THIS_ITEM(tree, hfindex, hfinfo) \
+	TRY_TO_FAKE_THIS_ITEM_OR_FREE(tree, hfindex, hfinfo, ((void)0))
+
+
+/** See inlined comments.
  @param pi the created protocol item we're about to return */
 #define TRY_TO_FAKE_THIS_REPR(pi) \
 	g_assert(pi);			\
@@ -182,6 +196,8 @@ proto_tree_set_bytes(field_info *fi, const guint8* start_ptr, gint length);
 static void
 proto_tree_set_bytes_tvb(field_info *fi, tvbuff_t *tvb, gint offset, gint length);
 static void
+proto_tree_set_bytes_gbytearray(field_info *fi, const GByteArray *value);
+static void
 proto_tree_set_time(field_info *fi, const nstime_t *value_ptr);
 static void
 proto_tree_set_string(field_info *fi, const char* value);
@@ -1955,6 +1971,133 @@ proto_tree_add_item(proto_tree *tree, int hfindex, tvbuff_t *tvb,
 	return proto_tree_add_item_new(tree, proto_registrar_get_nth(hfindex), tvb, start, length, encoding);
 }
 
+/* which FT_ types can use proto_tree_add_bytes_item() */
+static inline gboolean
+validate_proto_tree_add_bytes_ftype(const enum ftenum type)
+{
+	return (type == FT_BYTES      ||
+		type == FT_UINT_BYTES ||
+		type == FT_OID        ||
+		type == FT_REL_OID    ||
+		type == FT_SYSTEM_ID  );
+}
+
+/* Note: this does no validation that the byte array of an FT_OID or
+   FT_REL_OID is actually valid; and neither does proto_tree_add_item(),
+   so I think it's ok to continue not validating it?
+ */
+proto_item *
+proto_tree_add_bytes_item(proto_tree *tree, int hfindex, tvbuff_t *tvb,
+			   const gint start, gint length, const guint encoding,
+			   GByteArray *retval, gint *endoff, gint *err)
+{
+	field_info	  *new_fi;
+	GByteArray	  *bytes = retval;
+	GByteArray	  *created_bytes = NULL;
+	gint		   saved_err = 0;
+	guint32		   n = 0;
+	header_field_info *hfinfo = proto_registrar_get_nth(hfindex);
+	gboolean	   generate = (bytes || tree) ? TRUE : FALSE;
+
+	DISSECTOR_ASSERT_HINT(hfinfo != NULL, "Not passed hfi!");
+
+	DISSECTOR_ASSERT_HINT(validate_proto_tree_add_bytes_ftype(hfinfo->type),
+		"Called proto_tree_add_bytes_item but not a bytes-based FT_XXX type");
+
+	/* length has to be -1 or > 0 regardless of encoding */
+	/* invalid FT_UINT_BYTES length is caught in get_uint_value() */
+	if (length < -1 || length == 0) {
+		REPORT_DISSECTOR_BUG(wmem_strdup_printf(wmem_packet_scope(),
+		    "Invalid length %d passed to proto_tree_add_bytes_item for %s",
+		    length, ftype_name(hfinfo->type)));
+	}
+
+	if (encoding & ENC_STR_NUM) {
+		REPORT_DISSECTOR_BUG("Decoding number strings for byte arrays is not supported");
+	}
+
+	if (generate && (encoding & ENC_STR_HEX)) {
+		if (hfinfo->type == FT_UINT_BYTES) {
+			/* can't decode FT_UINT_BYTES from strings */
+			REPORT_DISSECTOR_BUG("proto_tree_add_bytes_item called for "
+			    "FT_UINT_BYTES type, but as ENC_STR_HEX");
+		}
+
+		if (!bytes) {
+			/* caller doesn't care about return value, but we need it to
+			   call tvb_get_string_bytes() and set the tree later */
+			bytes = created_bytes = g_byte_array_new();
+		}
+
+		/* bytes might be NULL after this, but can't add expert error until later */
+		bytes = tvb_get_string_bytes(tvb, start, length, encoding, bytes, endoff);
+
+		/* grab the errno now before it gets overwritten */
+		saved_err = errno;
+	}
+	else if (generate) {
+		tvb_ensure_bytes_exist(tvb, start, length);
+
+		if (!bytes) {
+			/* caller doesn't care about return value, but we need it to
+			   call tvb_get_string_bytes() and set the tree later */
+			bytes = created_bytes = g_byte_array_new();
+		}
+
+		if (hfinfo->type == FT_UINT_BYTES) {
+			n = length; /* n is now the "header" length */
+			length = get_uint_value(tree, tvb, start, n, encoding);
+			/* length is now the value's length; only store the value in the array */
+			g_byte_array_append(bytes, tvb_get_ptr(tvb, start + n, length), length);
+		}
+		else if (length > 0) {
+			g_byte_array_append(bytes, tvb_get_ptr(tvb, start, length), length);
+		}
+
+		if (endoff)
+		    *endoff = start + n + length;
+	}
+
+	if (err) *err = saved_err;
+
+	TRY_TO_FAKE_THIS_ITEM_OR_FREE(tree, hfinfo->id, hfinfo,
+		{
+		    if (created_bytes)
+			g_byte_array_free(created_bytes, TRUE);
+		    created_bytes = NULL;
+		    bytes = NULL;
+		} );
+
+	/* n will be zero except when it's a FT_UINT_BYTES */
+	new_fi = new_field_info(tree, hfinfo, tvb, start, n + length);
+
+	if (new_fi == NULL)
+		return NULL;
+
+	if (encoding & ENC_STRING) {
+		if (saved_err == ERANGE)
+		    expert_add_info(NULL, tree, &ei_number_string_decoding_erange_error);
+		else if (!bytes || saved_err != 0)
+		    expert_add_info(NULL, tree, &ei_number_string_decoding_failed_error);
+
+		if (bytes)
+		    proto_tree_set_bytes_gbytearray(new_fi, bytes);
+
+		if (created_bytes)
+		    g_byte_array_free(created_bytes, TRUE);
+	}
+	else {
+		/* n will be zero except when it's a FT_UINT_BYTES */
+		proto_tree_set_bytes_tvb(new_fi, tvb, start + n, length);
+
+		FI_SET_FLAG(new_fi,
+			(encoding & ENC_LITTLE_ENDIAN) ? FI_LITTLE_ENDIAN : FI_BIG_ENDIAN);
+	}
+
+	return proto_tree_add_node(tree, new_fi);
+}
+
+
 proto_item *
 proto_tree_add_time_item(proto_tree *tree, int hfindex, tvbuff_t *tvb,
 			   const gint start, gint length, const guint encoding,
@@ -2205,6 +2348,18 @@ proto_tree_set_bytes_tvb(field_info *fi, tvbuff_t *tvb, gint offset, gint length
 	proto_tree_set_bytes(fi, tvb_get_ptr(tvb, offset, length), length);
 }
 
+static void
+proto_tree_set_bytes_gbytearray(field_info *fi, const GByteArray *value)
+{
+	GByteArray *bytes;
+
+	DISSECTOR_ASSERT(value != NULL);
+
+	bytes = byte_array_dup(value);
+
+	fvalue_set_byte_array(&fi->value, bytes);
+}
+
 /* Add a FT_*TIME to a proto_tree */
 proto_item *
 proto_tree_add_time(proto_tree *tree, int hfindex, tvbuff_t *tvb, gint start,
diff --git a/epan/proto.h b/epan/proto.h
index d17242990e..eac413478a 100644
--- a/epan/proto.h
+++ b/epan/proto.h
@@ -347,6 +347,22 @@ WS_DLL_PUBLIC WS_MSVC_NORETURN void proto_report_dissector_bug(const char *messa
 /* mask out ENC_STR_* and related bits - should this replace ENC_CHARENCODING_MASK? */
 #define ENC_STR_MASK            0x0000FFFE
 
+/* for cases where the number is allowed to have a leading '+'/'-' */
+/* this can't collide with ENC_SEP_* because they can be used simultaneously */
+#define ENC_NUM_PREF    0x00200000
+
+/* For cases where a string encoding contains hex, bit-or one or more
+ * of these for the allowed separator(s), as well as with ENC_STR_HEX.
+ * See hex_str_to_bytes_encoding() in epan/strutil.h for details.
+ */
+#define ENC_SEP_NONE    0x00010000
+#define ENC_SEP_COLON   0x00020000
+#define ENC_SEP_DASH    0x00040000
+#define ENC_SEP_DOT   0x00080000
+#define ENC_SEP_SPACE   0x00100000
+/* a convenience macro for the above */
+#define ENC_SEP_MASK    0x001F0000
+
 /* For cases where a string encoding contains a timestamp, use one
  * of these (but only one). These values can collide with above, because
  * you can't do both at the same time.
@@ -905,6 +921,44 @@ WS_DLL_PUBLIC proto_item *
 proto_tree_add_bytes(proto_tree *tree, int hfindex, tvbuff_t *tvb, gint start,
 	gint length, const guint8* start_ptr);
 
+/** Get and add a byte-array-based FT_* to a proto_tree.
+
+ Supported: FT_BYTES, FT_UINT_BYTES, FT_OID, FT_REL_OID, and FT_SYSTEM_ID.
+
+ The item is extracted from the tvbuff handed to it, based on the ENC_* passed
+ in for the encoding, and the retrieved byte array is also set to *retval so the
+ caller gets it back for other uses.
+
+ This function retrieves the value even if the passed-in tree param is NULL,
+ so that it can be used by dissectors at all times to both get the value
+ and set the tree item to it.
+
+ Like other proto_tree_add functions, if there is a tree and the value cannot
+ be decoded from the tvbuff, then an expert info error is reported. For string
+ encoding, this means that a failure to decode the hex value from the string
+ results in an expert info error being added to the tree.
+
+ If encoding is string-based, it will convert using tvb_get_string_bytes(); see
+ that function's comments for details.
+
+ @note The GByteArray retval must be pre-constructed using g_byte_array_new().
+
+ @param tree the tree to append this item to
+ @param hfindex field index
+ @param tvb the tv buffer of the current data
+ @param start start of data in tvb
+ @param length length of data in tvb
+ @param encoding data encoding (e.g, ENC_LITTLE_ENDIAN, or ENC_UTF_8|ENC_STR_HEX)
+ @param[in,out] retval points to a GByteArray which will be set to the bytes from the Tvb.
+ @param[in,out] endoff if not NULL, gets set to the character after those consumed.
+ @param[in,out] err if not NULL, gets set to 0 if no failure, else the errno code (e.g., EDOM, ERANGE).
+ @return the newly created item, and retval is set to the decoded value
+ */
+WS_DLL_PUBLIC proto_item *
+proto_tree_add_bytes_item(proto_tree *tree, int hfindex, tvbuff_t *tvb,
+    const gint start, gint length, const guint encoding,
+    GByteArray *retval, gint *endoff, gint *err);
+
 /** Add a formatted FT_BYTES to a proto_tree, with the format generating
     the string for the value and with the field name being included
     automatically.
diff --git a/epan/strutil.c b/epan/strutil.c
index 567d072abd..d9b8c5a553 100644
--- a/epan/strutil.c
+++ b/epan/strutil.c
@@ -30,6 +30,7 @@
 #include "emem.h"
 
 #include <wsutil/str_util.h>
+#include <epan/proto.h>
 
 #ifdef _WIN32
 #include <windows.h>
@@ -545,6 +546,132 @@ hex_str_to_bytes(const char *hex_str, GByteArray *bytes, gboolean force_separato
     return TRUE;
 }
 
+static inline gchar
+get_valid_byte_sep(gchar c, const guint encoding)
+{
+    gchar retval = -1; /* -1 means failure */
+
+    switch (c) {
+        case ':':
+            if (encoding & ENC_SEP_COLON)
+                retval = c;
+            break;
+        case '-':
+            if (encoding & ENC_SEP_DASH)
+                retval = c;
+            break;
+        case '.':
+            if (encoding & ENC_SEP_DOT)
+                retval = c;
+            break;
+        case ' ':
+            if (encoding & ENC_SEP_SPACE)
+                retval = c;
+            break;
+        case '\0':
+            /* we were given the end of the string, so it's fine */
+            retval = 0;
+            break;
+        default:
+            if (isxdigit(c) && (encoding & ENC_SEP_NONE))
+                retval = 0;
+            /* anything else means we've got a failure */
+            break;
+    }
+
+    return retval;
+}
+
+/* Turn a string of hex digits with optional separators (defined by is_byte_sep())
+ * into a byte array. Unlike hex_str_to_bytes(), this will read as many hex-char
+ * pairs as possible and not error if it hits a non-hex-char; instead it just ends
+ * there. (i.e., like strtol()/atoi()/etc.) Unless fail_if_partial is TRUE.
+ *
+ * The **endptr, if not NULL, is set to the char after the last hex character.
+ */
+gboolean
+hex_str_to_bytes_encoding(const gchar *hex_str, GByteArray *bytes, const gchar **endptr,
+                          const guint encoding, const gboolean fail_if_partial)
+{
+    gchar c, d;
+    guint8 val;
+    const gchar *end = hex_str;
+    gboolean retval = FALSE;
+    gchar sep = -1;
+
+    /* a map from ASCII hex chars to their value */
+    static const gchar str_to_nibble[256] = {
+        -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
+        -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
+        -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
+         0, 1, 2, 3, 4, 5, 6, 7, 8, 9,-1,-1,-1,-1,-1,-1,
+        -1,10,11,12,13,14,15,-1,-1,-1,-1,-1,-1,-1,-1,-1,
+        -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
+        -1,10,11,12,13,14,15,-1,-1,-1,-1,-1,-1,-1,-1,-1,
+        -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
+        -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
+        -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
+        -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
+        -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
+        -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
+        -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
+        -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
+        -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1
+    };
+
+    /* we must see two hex chars at the beginning, or fail */
+    if (bytes && *end && isxdigit(*end) && isxdigit(*(end+1))) {
+        retval = TRUE;
+
+        /* set the separator character we'll allow; if this returns a -1, it means something's
+         * invalid after the hex, but we'll let the while-loop grab the first hex-pair anyway
+         */
+        sep = get_valid_byte_sep(*(end+2), encoding);
+
+        while (*end) {
+            c = str_to_nibble[(int)*end];
+            if (c < 0) {
+                if (fail_if_partial) retval = FALSE;
+                break;
+            }
+            ++end;
+
+            d = str_to_nibble[(int)*end];
+            if (d < 0) {
+                if (fail_if_partial) retval = FALSE;
+                break;
+            }
+            val = ((guint8)c * 16) + d;
+            g_byte_array_append(bytes, &val, 1);
+            ++end;
+
+            /* check for separator and peek at next char to make sure we should keep going */
+            if (sep > 0 && *end == sep && str_to_nibble[(int)*(end+1)] > -1) {
+                /* yes, it's the right sep and followed by more hex, so skip the sep */
+                ++end;
+            } else if (sep != 0 && *end) {
+                /* we either need a separator, but we don't see one; or the get_valid_byte_sep()
+                   earlier didn't find a valid one to begin with */
+                if (fail_if_partial) retval = FALSE;
+                break;
+            }
+            /* otherwise, either no separator allowed, or *end is null, or *end is an invalid
+             * sep, or *end is a valid sep but after it is not a hex char - in all those
+             * cases, just loop back up and let it fail later naturally.
+             */
+        }
+    }
+
+    if (!retval) {
+        if (bytes) g_byte_array_set_size(bytes, 0);
+        end = hex_str;
+    }
+
+    if (endptr) *endptr = end;
+
+    return retval;
+}
+
 /*
  * Turn an RFC 3986 percent-encoded string into a byte array.
  * XXX - We don't check for reserved characters.
@@ -666,7 +793,7 @@ format_uri(const GByteArray *bytes, const gchar *reserved_chars)
  *
  */
 GByteArray *
-byte_array_dup(GByteArray *ba) {
+byte_array_dup(const GByteArray *ba) {
     GByteArray *new_ba;
 
     if (!ba)
diff --git a/epan/strutil.h b/epan/strutil.h
index b9e8bdfcb7..f92136bbcd 100644
--- a/epan/strutil.h
+++ b/epan/strutil.h
@@ -112,6 +112,34 @@ WS_DLL_PUBLIC
 gboolean   hex_str_to_bytes(const char *hex_str, GByteArray *bytes,
     gboolean force_separators);
 
+/* Turn a string of hex digits with optional separators (defined by encoding)
+ * into a byte array. Unlike hex_str_to_bytes(), this will read as many hex-char
+ * pairs as possible and not error if it hits a non-hex-char; instead it just ends
+ * there. (i.e., like strtol()/atoi()/etc.) But it must see two hex chars at the
+ * beginning or it will return FALSE.
+ *
+ * @param hex_str The string of hex digits.
+ * @param bytes The GByteArray that will receive the bytes.  This
+ *        must be initialized by the caller.
+ * @param endptr if not NULL, is set to the char after the last hex character consumed.
+ * @param encoding set to one or more bitwise-or'ed ENC_SEP_* (see proto.h)
+ * @param fail_if_partial If set to TRUE, then the conversion fails if the whole
+ *    hex_str is not consumed.
+ * @return FALSE only if no bytes were generated; or if fail_if_partial is TRUE
+ *    and the entire hex_str was not consumed.
+ *
+ * If no ENC_SEP_* is set, then no separators are allowed. If multiple ENC_SEP_* are
+ * bit-or'ed, any of them can be a separator, but once the separator is seen then
+ * only its same type is accepted for the rest of the string. (i.e., it won't convert
+ * a "01:23-4567" even if ENC_SEP_COLON|ENC_SEP_DASH|ENC_SEP_NONE is passed in)
+ *
+ * This is done this way because it's likely a malformed scenario if they're mixed,
+ * and this routine is used by dissectors via tvb_get_string_XXX routines.
+ */
+WS_DLL_PUBLIC
+gboolean hex_str_to_bytes_encoding(const char *hex_str, GByteArray *bytes, const char **endptr,
+                                   const guint encoding, const gboolean fail_if_partial);
+
 /** Turn an RFC 3986 percent-encoded string into a byte array.
  *
  * @param uri_str The string of hex digits.
@@ -169,7 +197,7 @@ gboolean   oid_str_to_bytes(const char *oid_str, GByteArray *bytes);
  * @todo - Should this be in strutil.c?
  */
 WS_DLL_PUBLIC
-GByteArray *byte_array_dup(GByteArray *ba);
+GByteArray *byte_array_dup(const GByteArray *ba);
 
 /**
  * Compare the contents of two GByteArrays
diff --git a/epan/tvbuff.c b/epan/tvbuff.c
index 668d1c0f8b..8612921bd5 100644
--- a/epan/tvbuff.c
+++ b/epan/tvbuff.c
@@ -1307,6 +1307,36 @@ validate_single_byte_ascii_encoding(const guint encoding)
 	    REPORT_DISSECTOR_BUG("No string encoding type passed to tvb_get_string_XXX");
 }
 
+GByteArray*
+tvb_get_string_bytes(tvbuff_t *tvb, const gint offset, const gint length,
+		     const guint encoding, GByteArray *bytes, gint *endoff)
+{
+	const gchar *ptr = (gchar*) tvb_get_raw_string(wmem_packet_scope(), tvb, offset, length);
+	const gchar *begin = ptr;
+	const gchar *end = NULL;
+	GByteArray* retval = NULL;
+
+	errno = EDOM;
+
+	validate_single_byte_ascii_encoding(encoding);
+
+	if (endoff) *endoff = 0;
+
+	while (*begin == ' ') begin++;
+
+	if (*begin && bytes) {
+		if (hex_str_to_bytes_encoding(begin, bytes, &end, encoding, FALSE)) {
+			if (bytes->len > 0) {
+				if (endoff) *endoff = offset + (gint)(end - ptr);
+				errno = 0;
+				retval = bytes;
+			}
+		}
+	}
+
+	return retval;
+}
+
 /* converts a broken down date representation, relative to UTC,
  * to a timestamp; it uses timegm() if it's available.
  * Copied from Glib source gtimer.c
diff --git a/epan/tvbuff.h b/epan/tvbuff.h
index 14789d318b..e68095759a 100644
--- a/epan/tvbuff.h
+++ b/epan/tvbuff.h
@@ -367,6 +367,18 @@ WS_DLL_PUBLIC
 struct nstime_t* tvb_get_string_time(tvbuff_t *tvb, const gint offset, const gint length,
                               const guint encoding, struct nstime_t* ns, gint *endoff);
 
+/* Similar to above, but returns a GByteArray based on the case-insensitive
+ * hex-char strings with optional separators, and with optional leading spaces.
+ * The separators allowed are based on the ENC_SEP_* passed in the encoding param.
+ *
+ * The passed-in bytes is set to the values, and its pointer is also the return
+ * value or NULL on error. The GByteArray bytes must be pre-constructed with
+ * g_byte_array_new().
+ */
+WS_DLL_PUBLIC
+GByteArray* tvb_get_string_bytes(tvbuff_t *tvb, const gint offset, const gint length,
+                                 const guint encoding, GByteArray* bytes, gint *endoff);
+
 /**
  * Fetch an IPv4 address, in network byte order.
  * We do *not* convert it to host byte order; we leave it in
diff --git a/epan/wslua/wslua_field.c b/epan/wslua/wslua_field.c
index 0720edf591..72dc551d7f 100644
--- a/epan/wslua/wslua_field.c
+++ b/epan/wslua/wslua_field.c
@@ -75,7 +75,16 @@ WSLUA_METAMETHOD FieldInfo__unm(lua_State* L) {
 /* WSLUA_ATTRIBUTE FieldInfo_value RO The value of this field. */
 WSLUA_METAMETHOD FieldInfo__call(lua_State* L) {
     /*
-       Obtain the Value of the field
+       Obtain the Value of the field.
+
+       Previous to 1.11.4, this function retrieved the value for most field types,
+       but for `ftypes.UINT_BYTES` it retrieved the `ByteArray` of the field's entire `TvbRange`.
+       In other words, it returned a `ByteArray` that included the leading length byte(s),
+       instead of just the *value* bytes. That was a bug, and has been changed in 1.11.4.
+       Furthermore, it retrieved an `ftypes.GUID` as a `ByteArray`, which is also incorrect.
+
+       If you wish to still get a `ByteArray` of the `TvbRange`, use `FieldInfo:get_range()`
+       to get the `TvbRange`, and then use `Tvb:bytes()` to convert it to a `ByteArray`.
        */
     FieldInfo fi = checkFieldInfo(L,1);
 
@@ -165,16 +174,18 @@ WSLUA_METAMETHOD FieldInfo__call(lua_State* L) {
                 /* FALLTHROUGH */
         case FT_BYTES:
         case FT_UINT_BYTES:
-        case FT_GUID:
         case FT_PROTOCOL:
         case FT_REL_OID:
         case FT_SYSTEM_ID:
-        case FT_OID: {
+        case FT_OID:
+            {
                 ByteArray ba = g_byte_array_new();
-                g_byte_array_append(ba, (const guint8 *)tvb_memdup(wmem_packet_scope(),fi->ws_fi->ds_tvb,fi->ws_fi->start,fi->ws_fi->length),fi->ws_fi->length);
+                g_byte_array_append(ba, (const guint8 *) fvalue_get(&fi->ws_fi->value),
+                                    fvalue_length(&fi->ws_fi->value));
                 pushByteArray(L,ba);
                 return 1;
             }
+        case FT_GUID:
         default:
                 luaL_error(L,"FT_ not yet supported");
                 return 1;
diff --git a/epan/wslua/wslua_tree.c b/epan/wslua/wslua_tree.c
index 78c4390e41..afd5feef90 100644
--- a/epan/wslua/wslua_tree.c
+++ b/epan/wslua/wslua_tree.c
@@ -62,6 +62,25 @@ try_add_packet_field(lua_State *L, TreeItem tree_item, TvbRange tvbr, const int
     gint endoff = 0;
 
     switch(type) {
+        /* these all generate ByteArrays */
+        case FT_BYTES:
+        case FT_UINT_BYTES:
+        case FT_OID:
+        case FT_REL_OID:
+        case FT_SYSTEM_ID:
+            {
+                /* GByteArray and its data will be g_free'd by Lua */
+                GByteArray *gba = g_byte_array_new();
+                item = proto_tree_add_bytes_item(tree_item->tree, hfid, tvbr->tvb->ws_tvb,
+                                                   tvbr->offset, tvbr->len, encoding,
+                                                   gba, &endoff, &err);
+                if (err == 0) {
+                    pushByteArray(L, gba);
+                    lua_pushinteger(L, endoff);
+                }
+            }
+            break;
+
         case FT_ABSOLUTE_TIME:
         case FT_RELATIVE_TIME:
             {
@@ -77,6 +96,9 @@ try_add_packet_field(lua_State *L, TreeItem tree_item, TvbRange tvbr, const int
             }
             break;
 
+        /* XXX: what about these? */
+        case FT_NONE:
+        case FT_PROTOCOL:
         /* anything else just needs to be done the old fashioned way */
         default:
             item = proto_tree_add_item(tree_item->tree, hfid, tvbr->tvb->ws_tvb, tvbr->offset, tvbr->len, encoding);
diff --git a/epan/wslua/wslua_tvb.c b/epan/wslua/wslua_tvb.c
index a48e48f32c..d40de58436 100644
--- a/epan/wslua/wslua_tvb.c
+++ b/epan/wslua/wslua_tvb.c
@@ -106,6 +106,27 @@ WSLUA_METAMETHOD ByteArray__concat(lua_State* L) {
     WSLUA_RETURN(1); /* The new composite `ByteArray`. */
 }
 
+WSLUA_METAMETHOD ByteArray__eq(lua_State* L) {
+    /* Compares two ByteArray values.
+
+       @since 1.11.4
+     */
+#define WSLUA_ARG_ByteArray__eq_FIRST 1 /* First array. */
+#define WSLUA_ARG_ByteArray__eq_SECOND 2 /* Second array. */
+    ByteArray ba1 = checkByteArray(L,WSLUA_ARG_ByteArray__eq_FIRST);
+    ByteArray ba2 = checkByteArray(L,WSLUA_ARG_ByteArray__eq_SECOND);
+    gboolean result = FALSE;
+
+    if (ba1->len == ba2->len) {
+        if (memcmp(ba1->data, ba2->data, ba1->len) == 0)
+            result = TRUE;
+    }
+
+    lua_pushboolean(L,result);
+
+    return 1;
+}
+
 WSLUA_METHOD ByteArray_prepend(lua_State* L) {
 	/* Prepend a `ByteArray` to this `ByteArray`. */
 #define WSLUA_ARG_ByteArray_prepend_PREPENDED 2 /* `ByteArray` to be prepended. */
@@ -325,6 +346,7 @@ WSLUA_METHODS ByteArray_methods[] = {
 WSLUA_META ByteArray_meta[] = {
     WSLUA_CLASS_MTREG(ByteArray,tostring),
     WSLUA_CLASS_MTREG(ByteArray,concat),
+    WSLUA_CLASS_MTREG(ByteArray,eq),
     {"__call",ByteArray_subset},
     { NULL, NULL }
 };
@@ -1309,9 +1331,25 @@ WSLUA_METHOD TvbRange_le_ustringz(lua_State* L) {
 }
 
 WSLUA_METHOD TvbRange_bytes(lua_State* L) {
-	/* Obtain a `ByteArray` from a `TvbRange`. */
+	/* Obtain a `ByteArray` from a `TvbRange`.
+
+       Starting in 1.11.4, this function also takes an optional `encoding` argument,
+       which can be set to `ENC_STR_HEX` to decode a hex-string from the `TvbRange`
+       into the returned `ByteArray`. The `encoding` can be bitwise-or'ed with one
+       or more separator encodings, such as `ENC_SEP_COLON`, to allow separators
+       to occur between each pair of hex characters.
+
+       The return value also now returns the number of bytes used as a second return value.
+
+       On failure or error, nil is returned for both return values.
+
+       @note The encoding type of the hex string should also be set, for example
+       `ENC_ASCII` or `ENC_UTF_8`, along with `ENC_STR_HEX`.
+     */
+#define WSLUA_OPTARG_TvbRange_bytes_ENCODING 2 /* An optional ENC_* encoding value to use */
     TvbRange tvbr = checkTvbRange(L,1);
     GByteArray* ba;
+    const guint encoding = luaL_optint(L, WSLUA_OPTARG_TvbRange_bytes_ENCODING, 0);
 
     if ( !(tvbr && tvbr->tvb)) return 0;
     if (tvbr->tvb->expired) {
@@ -1320,11 +1358,32 @@ WSLUA_METHOD TvbRange_bytes(lua_State* L) {
     }
 
     ba = g_byte_array_new();
-    g_byte_array_append(ba,(const guint8 *)tvb_memdup(wmem_packet_scope(),tvbr->tvb->ws_tvb,tvbr->offset,tvbr->len),tvbr->len);
 
-    pushByteArray(L,ba);
+    if (encoding == 0) {
+        g_byte_array_append(ba,(const guint8 *)tvb_memdup(wmem_packet_scope(),tvbr->tvb->ws_tvb,tvbr->offset,tvbr->len),tvbr->len);
+        pushByteArray(L,ba);
+        lua_pushinteger(L, tvbr->len);
+    }
+    else if ((encoding & ENC_STR_HEX) == 0) {
+        WSLUA_OPTARG_ERROR(TvbRange_nstime, ENCODING, "invalid encoding value");
+    }
+    else {
+        gint endoff = 0;
+        GByteArray* retval = tvb_get_string_bytes(tvbr->tvb->ws_tvb, tvbr->offset, tvbr->len,
+                                                  encoding, ba, &endoff);
+        if (!retval || endoff == 0) {
+            g_byte_array_free(ba, TRUE);
+            /* push nil nstime and offset */
+            lua_pushnil(L);
+            lua_pushnil(L);
+        }
+        else {
+            pushByteArray(L,ba);
+            lua_pushinteger(L, endoff);
+        }
+    }
 
-    WSLUA_RETURN(1); /* The `ByteArray` object. */
+    WSLUA_RETURN(2); /* The `ByteArray` object or nil, and number of bytes consumed or nil. */
 }
 
 WSLUA_METHOD TvbRange_bitfield(lua_State* L) {