From 2de1124e364a4612c00a806d89923ced6869e3a6 Mon Sep 17 00:00:00 2001
From: Josh Coalson <jcoalson@users.sourceforce.net>
Date: Thu, 30 Dec 2004 03:41:19 +0000
Subject: [PATCH] add vorbiscomment validation functions to format.c and test
 code; validate name part of vorbiscomment in metadata and metadata++
 interfaces

---
 doc/html/changelog.html                |  10 +-
 include/FLAC++/metadata.h              |  32 ++--
 include/FLAC/format.h                  |  47 +++++-
 include/FLAC/metadata.h                |  39 +++--
 src/libFLAC++/metadata.cpp             |  19 ++-
 src/libFLAC/format.c                   |  71 ++++++++
 src/libFLAC/metadata_object.c          |  20 +++
 src/test_libFLAC++/metadata_object.cpp |   7 +-
 src/test_libFLAC/Makefile.am           |   2 +
 src/test_libFLAC/Makefile.lite         |   1 +
 src/test_libFLAC/format.c              | 222 +++++++++++++++++++++++++
 src/test_libFLAC/format.h              |  26 +++
 src/test_libFLAC/main.c                |   4 +
 src/test_libFLAC/test_libFLAC.dsp      |   8 +
 14 files changed, 466 insertions(+), 42 deletions(-)
 create mode 100644 src/test_libFLAC/format.c
 create mode 100644 src/test_libFLAC/format.h

diff --git a/doc/html/changelog.html b/doc/html/changelog.html
index 77168cb8..935d4c2e 100644
--- a/doc/html/changelog.html
+++ b/doc/html/changelog.html
@@ -138,7 +138,11 @@
 				<LI>
 					libFLAC:
 					<UL>
-						<LI>Metadata interface now maintains a trailing NULL on Vorbis comment entries for convenience.</LI>
+						<LI><B>Changed:</B> Metadata object interface now maintains a trailing NULL on Vorbis comment entries for convenience.</LI>
+						<LI><B>Changed:</B> Metadata object interface now validates all Vorbis comment entries on input and returns false if an entry does not conform to the Vorbis comment spec.</LI>
+						<LI><B>Added</B> FLAC__format_vorbiscomment_entry_name_is_legal()</LI>
+						<LI><B>Added</B> FLAC__format_vorbiscomment_entry_value_is_legal()</LI>
+						<LI><B>Added</B> FLAC__format_vorbiscomment_entry_is_legal()</LI>
 						<LI><B>Added</B> FLAC__metadata_object_vorbiscomment_entry_from_name_value_pair()</LI>
 						<LI><B>Added</B> FLAC__metadata_object_vorbiscomment_entry_to_name_value_pair()</LI>
 						<LI><B>Changed</B> the signature of FLAC__metadata_object_vorbiscomment_entry_matches(): the first argument is now <TT>FLAC__StreamMetadata_VorbisComment_Entry entry</TT> (was <TT>const FLAC__StreamMetadata_VorbisComment_Entry *entry</TT>), i.e. <TT>entry</TT> is now pass-by-value.</LI>
@@ -147,7 +151,8 @@
 				<LI>
 					libFLAC++:
 					<UL>
-						<LI>Metadata interface now maintains a trailing NULL on Vorbis comment values for convenience.</LI>
+						<LI><B>Changed:</B> Metadata object interface now maintains a trailing NULL on Vorbis comment values for convenience.</LI>
+						<LI><B>Changed:</B> Metadata object interface now validates all Vorbis comment entries on input and returns false if an entry does not conform to the Vorbis comment spec.</LI>
 						<LI><B>Added</B> methods to FLAC::Metadata::VorbisComment::Entry for setting comment values from null-terminated strings:
 							<UL>
 								<LI>Entry(const char *field)</LI>
@@ -156,6 +161,7 @@
 								<LI>bool set_field_value(const char *field_value)</LI>
 							</UL>
 						</LI>
+						<LI><B>Changed</B> the signature of FLAC::Metadata::VorbisComment::get_vendor_string() and FLAC::Metadata::VorbisComment::set_vendor_string() to use a UTF-8, NUL-terminated string <TT>const FLAC__byte *</TT> for the vendor string instead of <TT>FLAC::Metadata::VorbisComment::Entry</TT>.</LI>
 					</UL>
 				</LI>
 				<LI>
diff --git a/include/FLAC++/metadata.h b/include/FLAC++/metadata.h
index 138580e0..78eb868d 100644
--- a/include/FLAC++/metadata.h
+++ b/include/FLAC++/metadata.h
@@ -493,36 +493,37 @@ namespace FLAC {
 			 *
 			 *  A \a field as used in the methods refers to an
 			 *  entire 'NAME=VALUE' string; for convenience the
-			 *  string is null-terminated.  A length field is
+			 *  string is NUL-terminated.  A length field is
 			 *  required in the unlikely event that the value
-			 *  contains contain embedded nulls.
+			 *  contains contain embedded NULs.
 			 *
 			 *  A \a field_name is what is on the left side of the
 			 *  first '=' in the \a field.  By definition it is ASCII
-			 *  and so is null-terminated and does not require a
+			 *  and so is NUL-terminated and does not require a
 			 *  length to describe it.  \a field_name is undefined
 			 *  for a vendor string entry.
 			 *
 			 *  A \a field_value is what is on the right side of the
 			 *  first '=' in the \a field.  By definition, this may
-			 *  contain embedded nulls and so a \a field_value_length
+			 *  contain embedded NULs and so a \a field_value_length
 			 *  is required to describe it.  However in practice,
-			 *  embedded nulls are not known to be used, so it is
-			 *  generally safe to treat field values as null-
+			 *  embedded NULs are not known to be used, so it is
+			 *  generally safe to treat field values as NUL-
 			 *  terminated UTF-8 strings.
 			 *
 			 *  Always check is_valid() after the constructor or operator=
-			 *  to make sure memory was properly allocated.
+			 *  to make sure memory was properly allocated and that the
+			 *  Entry conforms to the Vorbis comment specification.
 			 */
 			class FLACPP_API Entry {
 			public:
 				Entry();
 
 				Entry(const char *field, unsigned field_length);
-				Entry(const char *field); // assumes \a field is null-terminated
+				Entry(const char *field); // assumes \a field is NUL-terminated
 
 				Entry(const char *field_name, const char *field_value, unsigned field_value_length);
-				Entry(const char *field_name, const char *field_value); // assumes \a field_value is null-terminated
+				Entry(const char *field_name, const char *field_value); // assumes \a field_value is NUL-terminated
 
 				Entry(const Entry &entry);
 
@@ -542,10 +543,10 @@ namespace FLAC {
 				const char *get_field_value() const;
 
 				bool set_field(const char *field, unsigned field_length);
-				bool set_field(const char *field); // assumes \a field is null-terminated
+				bool set_field(const char *field); // assumes \a field is NUL-terminated
 				bool set_field_name(const char *field_name);
 				bool set_field_value(const char *field_value, unsigned field_value_length);
-				bool set_field_value(const char *field_value); // assumes \a field_value is null-terminated
+				bool set_field_value(const char *field_value); // assumes \a field_value is NUL-terminated
 			protected:
 				bool is_valid_;
 				::FLAC__StreamMetadata_VorbisComment_Entry entry_;
@@ -560,9 +561,9 @@ namespace FLAC {
 				void clear_field_name();
 				void clear_field_value();
 				void construct(const char *field, unsigned field_length);
-				void construct(const char *field); // assumes \a field is null-terminated
+				void construct(const char *field); // assumes \a field is NUL-terminated
 				void construct(const char *field_name, const char *field_value, unsigned field_value_length);
-				void construct(const char *field_name, const char *field_value); // assumes \a field_value is null-terminated
+				void construct(const char *field_name, const char *field_value); // assumes \a field_value is NUL-terminated
 				void compose_field();
 				void parse_field();
 			};
@@ -607,12 +608,11 @@ namespace FLAC {
 			//@}
 
 			unsigned get_num_comments() const;
-			Entry get_vendor_string() const; // only the Entry's field name should be used
+			const FLAC__byte *get_vendor_string() const; // NUL-terminated UTF-8 string
 			Entry get_comment(unsigned index) const;
 
 			//! See FLAC__metadata_object_vorbiscomment_set_vendor_string()
-			//! \note Only the Entry's field name will be used.
-			bool set_vendor_string(const Entry &entry);
+			bool set_vendor_string(const FLAC__byte *string); // NUL-terminated UTF-8 string
 
 			//! See FLAC__metadata_object_vorbiscomment_set_comment()
 			bool set_comment(unsigned index, const Entry &entry);
diff --git a/include/FLAC/format.h b/include/FLAC/format.h
index 96bbbe04..1effad34 100644
--- a/include/FLAC/format.h
+++ b/include/FLAC/format.h
@@ -747,7 +747,6 @@ extern FLAC_API const unsigned FLAC__STREAM_METADATA_LENGTH_LEN; /**< == 24 (bit
  *
  *****************************************************************************/
 
-/* @@@@ add to unit tests; it is already indirectly tested by the metadata_object tests */
 /** Tests that a sample rate is valid for FLAC.  Since the rules for valid
  *  sample rates are slightly complex, they are encapsulated in this function.
  *
@@ -758,6 +757,52 @@ extern FLAC_API const unsigned FLAC__STREAM_METADATA_LENGTH_LEN; /**< == 24 (bit
  */
 FLAC_API FLAC__bool FLAC__format_sample_rate_is_valid(unsigned sample_rate);
 
+/** Check a Vorbis comment entry name to see if it conforms to the Vorbis
+ *  comment specification.
+ *
+ *  Vorbis comment names must be composed only of characters from
+ *  [0x20-0x3C,0x3E-0x7D].
+ *
+ * \param name       A NUL-terminated string to be checked.
+ * \assert
+ *    \code name != NULL \endcode
+ * \retval FLAC__bool
+ *    \c false if entry name is illegal, else \c true.
+ */
+FLAC_API FLAC__bool FLAC__format_vorbiscomment_entry_name_is_legal(const char *name);
+
+/** Check a Vorbis comment entry value to see if it conforms to the Vorbis
+ *  comment specification.
+ *
+ *  Vorbis comment values must be valid UTF-8 sequences.
+ *
+ * \param value      A string to be checked.
+ * \param length     A the length of \a value in bytes.  May be
+ *                   \c (unsigned)(-1) to indicate that \a value is a plain
+ *                   UTF-8 NUL-terminated string.
+ * \assert
+ *    \code value != NULL \endcode
+ * \retval FLAC__bool
+ *    \c false if entry name is illegal, else \c true.
+ */
+FLAC_API FLAC__bool FLAC__format_vorbiscomment_entry_value_is_legal(const FLAC__byte *value, unsigned length);
+
+/** Check a Vorbis comment entry to see if it conforms to the Vorbis
+ *  comment specification.
+ *
+ *  Vorbis comment entries must be of the form 'name=value', and 'name' and
+ *  'value' must be legal according to
+ *  FLAC__format_vorbiscomment_entry_name_is_legal() and
+ *  FLAC__format_vorbiscomment_entry_value_is_legal() respectively.
+ *
+ * \param value      A string to be checked.
+ * \assert
+ *    \code value != NULL \endcode
+ * \retval FLAC__bool
+ *    \c false if entry name is illegal, else \c true.
+ */
+FLAC_API FLAC__bool FLAC__format_vorbiscomment_entry_is_legal(const FLAC__byte *entry, unsigned length);
+
 /* @@@@ add to unit tests; it is already indirectly tested by the metadata_object tests */
 /** Check a seek table to see if it conforms to the FLAC specification.
  *  See the format specification for limits on the contents of the
diff --git a/include/FLAC/metadata.h b/include/FLAC/metadata.h
index 76893aff..6c831876 100644
--- a/include/FLAC/metadata.h
+++ b/include/FLAC/metadata.h
@@ -1087,13 +1087,17 @@ FLAC_API FLAC__bool FLAC__metadata_iterator_insert_block_after(FLAC__Metadata_It
  * FLAC__metadata_object_application_set_data(), you will get an assertion
  * failure.
  *
- * The FLAC__metadata_object_vorbiscomment_*() functions for convenience
+ * For convenience the FLAC__metadata_object_vorbiscomment_*() functions
  * maintain a trailing NUL on each Vorbis comment entry.  This is not counted
  * toward the length or stored in the stream, but it can make working with plain
  * comments (those that don't contain embedded-NULs in the value) easier.
  * Entries passed into these functions have trailing NULs added if missing, and
  * returned entries are guaranteed to have a trailing NUL.
  *
+ * The FLAC__metadata_object_vorbiscomment_*() functions that take a Vorbis
+ * comment entry/name/value will first validate that it complies with the Vorbis
+ * comment specification and return false if it does not.
+ *
  * There is no need to recalculate the length field on metadata blocks you
  * have modified.  They will be calculated automatically before they  are
  * written back to a file.
@@ -1357,7 +1361,8 @@ FLAC_API FLAC__bool FLAC__metadata_object_seektable_template_sort(FLAC__StreamMe
  *    \code (entry.entry != NULL && entry.length > 0) ||
  * (entry.entry == NULL && entry.length == 0) \endcode
  * \retval FLAC__bool
- *    \c false if memory allocation fails, else \c true.
+ *    \c false if memory allocation fails or \a entry does not comply with the
+ *    Vorbis comment specification, else \c true.
  */
 FLAC_API FLAC__bool FLAC__metadata_object_vorbiscomment_set_vendor_string(FLAC__StreamMetadata *object, FLAC__StreamMetadata_VorbisComment_Entry entry, FLAC__bool copy);
 
@@ -1374,7 +1379,7 @@ FLAC_API FLAC__bool FLAC__metadata_object_vorbiscomment_set_vendor_string(FLAC__
  *    \code (object->data.vorbis_comment.comments == NULL && object->data.vorbis_comment.num_comments == 0) ||
  * (object->data.vorbis_comment.comments != NULL && object->data.vorbis_comment.num_comments > 0) \endcode
  * \retval FLAC__bool
- *    \c false if memory allocation error, else \c true.
+ *    \c false if memory allocation fails, else \c true.
  */
 FLAC_API FLAC__bool FLAC__metadata_object_vorbiscomment_resize_comments(FLAC__StreamMetadata *object, unsigned new_num_comments);
 
@@ -1400,7 +1405,8 @@ FLAC_API FLAC__bool FLAC__metadata_object_vorbiscomment_resize_comments(FLAC__St
  *    \code (entry.entry != NULL && entry.length > 0) ||
  * (entry.entry == NULL && entry.length == 0) \endcode
  * \retval FLAC__bool
- *    \c false if memory allocation fails, else \c true.
+ *    \c false if memory allocation fails or \a entry does not comply with the
+ *    Vorbis comment specification, else \c true.
  */
 FLAC_API FLAC__bool FLAC__metadata_object_vorbiscomment_set_comment(FLAC__StreamMetadata *object, unsigned comment_num, FLAC__StreamMetadata_VorbisComment_Entry entry, FLAC__bool copy);
 
@@ -1429,7 +1435,8 @@ FLAC_API FLAC__bool FLAC__metadata_object_vorbiscomment_set_comment(FLAC__Stream
  *    \code (entry.entry != NULL && entry.length > 0) ||
  * (entry.entry == NULL && entry.length == 0 && copy == false) \endcode
  * \retval FLAC__bool
- *    \c false if memory allocation fails, else \c true.
+ *    \c false if memory allocation fails or \a entry does not comply with the
+ *    Vorbis comment specification, else \c true.
  */
 FLAC_API FLAC__bool FLAC__metadata_object_vorbiscomment_insert_comment(FLAC__StreamMetadata *object, unsigned comment_num, FLAC__StreamMetadata_VorbisComment_Entry entry, FLAC__bool copy);
 
@@ -1453,7 +1460,8 @@ FLAC_API FLAC__bool FLAC__metadata_object_vorbiscomment_insert_comment(FLAC__Str
  *    \code (entry.entry != NULL && entry.length > 0) ||
  * (entry.entry == NULL && entry.length == 0 && copy == false) \endcode
  * \retval FLAC__bool
- *    \c false if memory allocation fails, else \c true.
+ *    \c false if memory allocation fails or \a entry does not comply with the
+ *    Vorbis comment specification, else \c true.
  */
 FLAC_API FLAC__bool FLAC__metadata_object_vorbiscomment_append_comment(FLAC__StreamMetadata *object, FLAC__StreamMetadata_VorbisComment_Entry entry, FLAC__bool copy);
 
@@ -1487,7 +1495,8 @@ FLAC_API FLAC__bool FLAC__metadata_object_vorbiscomment_append_comment(FLAC__Str
  *    \code (entry.entry != NULL && entry.length > 0) ||
  * (entry.entry == NULL && entry.length == 0 && copy == false) \endcode
  * \retval FLAC__bool
- *    \c false if memory allocation fails, else \c true.
+ *    \c false if memory allocation fails or \a entry does not comply with the
+ *    Vorbis comment specification, else \c true.
  */
 FLAC_API FLAC__bool FLAC__metadata_object_vorbiscomment_replace_comment(FLAC__StreamMetadata *object, FLAC__StreamMetadata_VorbisComment_Entry entry, FLAC__bool all, FLAC__bool copy);
 
@@ -1513,14 +1522,15 @@ FLAC_API FLAC__bool FLAC__metadata_object_vorbiscomment_delete_comment(FLAC__Str
  * \param entry              A pointer to a Vorbis comment entry.  The entry's
  *                           \c entry pointer should not point to allocated
  *                           memory as it will be overwritten.
- * \param field_name         The field name in ASCII, \c NULL terminated.
- * \param field_value        The field value in UTF-8, \c NULL terminated.
+ * \param field_name         The field name in ASCII, \c NUL terminated.
+ * \param field_value        The field value in UTF-8, \c NUL terminated.
  * \assert
  *    \code entry != NULL \endcode
  *    \code field_name != NULL \endcode
  *    \code field_value != NULL \endcode
  * \retval FLAC__bool
- *    \c false if malloc() fails, else \c true.
+ *    \c false if malloc() fails, or if \a field_name or \a field_value does
+ *    not comply with the Vorbis comment specification, else \c true.
  */
 FLAC_API FLAC__bool FLAC__metadata_object_vorbiscomment_entry_from_name_value_pair(FLAC__StreamMetadata_VorbisComment_Entry *entry, const char *field_name, const char *field_value);
 
@@ -1529,7 +1539,7 @@ FLAC_API FLAC__bool FLAC__metadata_object_vorbiscomment_entry_from_name_value_pa
  *  The returned pointers to name and value will be allocated by malloc()
  *  and shall be owned by the caller.
  *
- * \param entry              A pointer to an existing Vorbis comment entry.
+ * \param entry              An existing Vorbis comment entry.
  * \param field_name         The address of where the returned pointer to the
  *                           field name will be stored.
  * \param field_value        The address of where the returned pointer to the
@@ -1540,17 +1550,18 @@ FLAC_API FLAC__bool FLAC__metadata_object_vorbiscomment_entry_from_name_value_pa
  *    \code field_name != NULL \endcode
  *    \code field_value != NULL \endcode
  * \retval FLAC__bool
- *    \c false if malloc() fails, else \c true.
+ *    \c false if memory allocation fails or \a entry does not comply with the
+ *    Vorbis comment specification, else \c true.
  */
 FLAC_API FLAC__bool FLAC__metadata_object_vorbiscomment_entry_to_name_value_pair(FLAC__StreamMetadata_VorbisComment_Entry entry, char **field_name, char **field_value);
 
 /** Check if the given Vorbis comment entry's field name matches the given
  *  field name.
  *
- * \param entry              A pointer to an existing Vorbis comment entry.
+ * \param entry              An existing Vorbis comment entry.
  * \param field_name         The field name to check.
  * \param field_name_length  The length of \a field_name, not including the
- *                           terminating \c NULL.
+ *                           terminating \c NUL.
  * \assert
  *    \code (entry.entry != NULL && entry.length > 0) \endcode
  * \retval FLAC__bool
diff --git a/src/libFLAC++/metadata.cpp b/src/libFLAC++/metadata.cpp
index 871b01e0..f5678df2 100644
--- a/src/libFLAC++/metadata.cpp
+++ b/src/libFLAC++/metadata.cpp
@@ -550,6 +550,9 @@ namespace FLAC {
 			FLAC__ASSERT(is_valid());
 			FLAC__ASSERT(0 != field);
 
+			if(!::FLAC__format_vorbiscomment_entry_is_legal((const ::FLAC__byte*)field, field_length))
+				return is_valid_ = false;
+
 			clear_entry();
 
 			if(0 == (entry_.entry = (FLAC__byte*)malloc(field_length+1))) {
@@ -575,6 +578,9 @@ namespace FLAC {
 			FLAC__ASSERT(is_valid());
 			FLAC__ASSERT(0 != field_name);
 
+			if(!::FLAC__format_vorbiscomment_entry_name_is_legal(field_name))
+				return is_valid_ = false;
+
 			clear_field_name();
 
 			if(0 == (field_name_ = strdup(field_name))) {
@@ -593,6 +599,9 @@ namespace FLAC {
 			FLAC__ASSERT(is_valid());
 			FLAC__ASSERT(0 != field_value);
 
+			if(!::FLAC__format_vorbiscomment_entry_value_is_legal((const FLAC__byte*)field_value, field_value_length))
+				return is_valid_ = false;
+
 			clear_field_value();
 
 			if(0 == (field_value_ = (char *)malloc(field_value_length+1))) {
@@ -756,10 +765,10 @@ namespace FLAC {
 			return object_->data.vorbis_comment.num_comments;
 		}
 
-		VorbisComment::Entry VorbisComment::get_vendor_string() const
+		const FLAC__byte *VorbisComment::get_vendor_string() const
 		{
 			FLAC__ASSERT(is_valid());
-			return Entry((const char *)object_->data.vorbis_comment.vendor_string.entry, object_->data.vorbis_comment.vendor_string.length);
+			return object_->data.vorbis_comment.vendor_string.entry;
 		}
 
 		VorbisComment::Entry VorbisComment::get_comment(unsigned index) const
@@ -769,13 +778,11 @@ namespace FLAC {
 			return Entry((const char *)object_->data.vorbis_comment.comments[index].entry, object_->data.vorbis_comment.comments[index].length);
 		}
 
-		bool VorbisComment::set_vendor_string(const VorbisComment::Entry &entry)
+		bool VorbisComment::set_vendor_string(const FLAC__byte *string)
 		{
 			FLAC__ASSERT(is_valid());
 			// vendor_string is a special kind of entry
-			::FLAC__StreamMetadata_VorbisComment_Entry vendor_string;
-			vendor_string.length = entry.get_field_name_length();
-			vendor_string.entry = (FLAC__byte*)entry.get_field_name(); // we can cheat on const-ness because we make a copy below:
+			const ::FLAC__StreamMetadata_VorbisComment_Entry vendor_string = { strlen((const char *)string), (FLAC__byte*)string }; // we can cheat on const-ness because we make a copy below:
 			return (bool)::FLAC__metadata_object_vorbiscomment_set_vendor_string(object_, vendor_string, /*copy=*/true);
 		}
 
diff --git a/src/libFLAC/format.c b/src/libFLAC/format.c
index d4ac5acb..d149928c 100644
--- a/src/libFLAC/format.c
+++ b/src/libFLAC/format.c
@@ -254,6 +254,77 @@ FLAC_API unsigned FLAC__format_seektable_sort(FLAC__StreamMetadata_SeekTable *se
 	return j;
 }
 
+static __inline unsigned utf8len_(const FLAC__byte *utf8)
+{
+	FLAC__ASSERT(0 != utf8);
+	if ((utf8[0] & 0x80) == 0)
+		return 1;
+	else if ((utf8[0] & 0xE0) == 0xC0 && (utf8[1] & 0xC0) == 0x80)
+		return 2;
+	else if ((utf8[0] & 0xF0) == 0xE0 && (utf8[1] & 0xC0) == 0x80 && (utf8[2] & 0xC0) == 0x80)
+		return 3;
+	else
+		return 0;
+}
+
+FLAC_API FLAC__bool FLAC__format_vorbiscomment_entry_name_is_legal(const char *name)
+{
+	char c;
+	for(c = *name; c; c = *(++name))
+		if(c < 0x20 || c == 0x3d || c > 0x7d)
+			return false;
+	return true;
+}
+
+FLAC_API FLAC__bool FLAC__format_vorbiscomment_entry_value_is_legal(const FLAC__byte *value, unsigned length)
+{
+	if(length == (unsigned)(-1)) {
+		while(*value) {
+			unsigned n = utf8len_(value);
+			if(n == 0)
+				return false;
+			value += n;
+		}
+	}
+	else {
+		const FLAC__byte *end = value + length;
+		while(value < end) {
+			unsigned n = utf8len_(value);
+			if(n == 0)
+				return false;
+			value += n;
+		}
+		if(value != end)
+			return false;
+	}
+	return true;
+}
+
+FLAC_API FLAC__bool FLAC__format_vorbiscomment_entry_is_legal(const FLAC__byte *entry, unsigned length)
+{
+	const FLAC__byte *s, *end;
+
+	for(s = entry, end = s + length; s < end && *s != '='; s++) {
+		if(*s < 0x20 || *s > 0x7D)
+			return false;
+	}
+	if(s == end)
+		return false;
+
+	s++; /* skip '=' */
+
+	while(s < end) {
+		unsigned n = utf8len_(s);
+		if(n == 0)
+			return false;
+		s += n;
+	}
+	if(s != end)
+		return false;
+
+	return true;
+}
+
 FLAC_API FLAC__bool FLAC__format_cuesheet_is_legal(const FLAC__StreamMetadata_CueSheet *cue_sheet, FLAC__bool check_cd_da_subset, const char **violation)
 {
 	unsigned i, j;
diff --git a/src/libFLAC/metadata_object.c b/src/libFLAC/metadata_object.c
index 3a613a1a..8907bdd8 100644
--- a/src/libFLAC/metadata_object.c
+++ b/src/libFLAC/metadata_object.c
@@ -964,6 +964,8 @@ FLAC_API FLAC__bool FLAC__metadata_object_seektable_template_sort(FLAC__StreamMe
 
 FLAC_API FLAC__bool FLAC__metadata_object_vorbiscomment_set_vendor_string(FLAC__StreamMetadata *object, FLAC__StreamMetadata_VorbisComment_Entry entry, FLAC__bool copy)
 {
+	if(!FLAC__format_vorbiscomment_entry_value_is_legal(entry.entry, entry.length))
+		return false;
 	return vorbiscomment_set_entry_(object, &object->data.vorbis_comment.vendor_string, &entry, copy);
 }
 
@@ -1016,6 +1018,8 @@ FLAC_API FLAC__bool FLAC__metadata_object_vorbiscomment_set_comment(FLAC__Stream
 	FLAC__ASSERT(0 != object);
 	FLAC__ASSERT(comment_num < object->data.vorbis_comment.num_comments);
 
+	if(!FLAC__format_vorbiscomment_entry_is_legal(entry.entry, entry.length))
+		return false;
 	return vorbiscomment_set_entry_(object, &object->data.vorbis_comment.comments[comment_num], &entry, copy);
 }
 
@@ -1027,6 +1031,9 @@ FLAC_API FLAC__bool FLAC__metadata_object_vorbiscomment_insert_comment(FLAC__Str
 	FLAC__ASSERT(object->type == FLAC__METADATA_TYPE_VORBIS_COMMENT);
 	FLAC__ASSERT(comment_num <= object->data.vorbis_comment.num_comments);
 
+	if(!FLAC__format_vorbiscomment_entry_is_legal(entry.entry, entry.length))
+		return false;
+
 	vc = &object->data.vorbis_comment;
 
 	if(!FLAC__metadata_object_vorbiscomment_resize_comments(object, vc->num_comments+1))
@@ -1050,6 +1057,10 @@ FLAC_API FLAC__bool FLAC__metadata_object_vorbiscomment_append_comment(FLAC__Str
 FLAC_API FLAC__bool FLAC__metadata_object_vorbiscomment_replace_comment(FLAC__StreamMetadata *object, FLAC__StreamMetadata_VorbisComment_Entry entry, FLAC__bool all, FLAC__bool copy)
 {
 	FLAC__ASSERT(0 != entry.entry && entry.length > 0);
+
+	if(!FLAC__format_vorbiscomment_entry_is_legal(entry.entry, entry.length))
+		return false;
+
 	{
 		int i;
 		unsigned field_name_length;
@@ -1111,6 +1122,11 @@ FLAC_API FLAC__bool FLAC__metadata_object_vorbiscomment_entry_from_name_value_pa
 	FLAC__ASSERT(0 != field_name);
 	FLAC__ASSERT(0 != field_value);
 
+	if(!FLAC__format_vorbiscomment_entry_name_is_legal(field_name))
+		return false;
+	if(!FLAC__format_vorbiscomment_entry_value_is_legal(field_value, (unsigned)(-1)))
+		return false;
+
 	{
 		const size_t nn = strlen(field_name);
 		const size_t nv = strlen(field_value);
@@ -1131,6 +1147,10 @@ FLAC_API FLAC__bool FLAC__metadata_object_vorbiscomment_entry_to_name_value_pair
 	FLAC__ASSERT(0 != entry.entry && entry.length > 0);
 	FLAC__ASSERT(0 != field_name);
 	FLAC__ASSERT(0 != field_value);
+
+	if(!FLAC__format_vorbiscomment_entry_is_legal(entry.entry, entry.length))
+		return false;
+
 	{
 		const FLAC__byte *eq = (FLAC__byte*)memchr(entry.entry, '=', entry.length);
 		const size_t nn = eq-entry.entry;
diff --git a/src/test_libFLAC++/metadata_object.cpp b/src/test_libFLAC++/metadata_object.cpp
index 97efbb67..9298c5aa 100644
--- a/src/test_libFLAC++/metadata_object.cpp
+++ b/src/test_libFLAC++/metadata_object.cpp
@@ -1018,14 +1018,15 @@ bool test_metadata_object_vorbiscomment()
 	printf("OK\n");
 
 	printf("testing VorbisComment::set_vendor_string()... ");
-	if(!block.set_vendor_string(entry1))
+	if(!block.set_vendor_string((const FLAC__byte *)"mame0"))
 		return die_("returned false");
 	printf("OK\n");
+	vorbiscomment_.data.vorbis_comment.vendor_string.entry[0] = 'm';
 
 	printf("testing VorbisComment::get_vendor_string()... ");
-	if(block.get_vendor_string().get_field_name_length() != vorbiscomment_.data.vorbis_comment.vendor_string.length)
+	if(strlen((const char *)block.get_vendor_string()) != vorbiscomment_.data.vorbis_comment.vendor_string.length)
 		return die_("length mismatch");
-	if(0 != memcmp(block.get_vendor_string().get_field_name(), vorbiscomment_.data.vorbis_comment.vendor_string.entry, vorbiscomment_.data.vorbis_comment.vendor_string.length))
+	if(0 != memcmp(block.get_vendor_string(), vorbiscomment_.data.vorbis_comment.vendor_string.entry, vorbiscomment_.data.vorbis_comment.vendor_string.length))
 		return die_("value mismatch");
 	printf("OK\n");
 
diff --git a/src/test_libFLAC/Makefile.am b/src/test_libFLAC/Makefile.am
index f3e47ac6..9036cbd4 100644
--- a/src/test_libFLAC/Makefile.am
+++ b/src/test_libFLAC/Makefile.am
@@ -33,6 +33,7 @@ test_libFLAC_SOURCES = \
 	decoders.c \
 	encoders.c \
 	file_utils.c \
+	format.c \
 	main.c \
 	metadata.c \
 	metadata_manip.c \
@@ -42,5 +43,6 @@ test_libFLAC_SOURCES = \
 	decoders.h \
 	encoders.h \
 	file_utils.h \
+	format.h \
 	metadata.h \
 	metadata_utils.h
diff --git a/src/test_libFLAC/Makefile.lite b/src/test_libFLAC/Makefile.lite
index 65f6e3d7..f01071d2 100644
--- a/src/test_libFLAC/Makefile.lite
+++ b/src/test_libFLAC/Makefile.lite
@@ -37,6 +37,7 @@ SRCS_C = \
 	decoders.c \
 	encoders.c \
 	file_utils.c \
+	format.c \
 	main.c \
 	metadata.c \
 	metadata_manip.c \
diff --git a/src/test_libFLAC/format.c b/src/test_libFLAC/format.c
new file mode 100644
index 00000000..75d8a310
--- /dev/null
+++ b/src/test_libFLAC/format.c
@@ -0,0 +1,222 @@
+/* test_libFLAC - Unit tester for libFLAC
+ * Copyright (C) 2004  Josh Coalson
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
+ */
+
+#include "FLAC/assert.h"
+#include "FLAC/format.h"
+#include <stdio.h>
+
+static const char *true_false_string_[2] = { "false", "true" };
+
+static struct {
+	unsigned rate;
+	FLAC__bool valid;
+} SAMPLE_RATES[] = {
+	{ 0, false },
+	{ 1, true },
+	{ 9, true },
+	{ 10, true },
+	{ 4000, true },
+	{ 8000, true },
+	{ 11025, true },
+	{ 12000, true },
+	{ 16000, true },
+	{ 22050, true },
+	{ 24000, true },
+	{ 32000, true },
+	{ 32768, true },
+	{ 44100, true },
+	{ 48000, true },
+	{ 65000, true },
+	{ 65535, true },
+	{ 65536, false },
+	{ 65540, true },
+	{ 65550, true },
+	{ 65555, false },
+	{ 66000, true },
+	{ 66001, false },
+	{ 96000, true },
+	{ 100000, true },
+	{ 100001, false },
+	{ 192000, true },
+	{ 500000, true },
+	{ 500001, false },
+	{ 500010, true },
+	{ 700000, false },
+	{ 700010, false },
+	{ 1000000, false },
+	{ 1100000, false }
+};
+
+static struct {
+	const char *string;
+	FLAC__bool valid;
+} VCENTRY_NAMES[] = {
+	{ "", true },
+	{ "a", true },
+	{ "=", false },
+	{ "a=", false },
+	{ "\x01", false },
+	{ "\x1f", false },
+	{ "\x7d", true },
+	{ "\x7e", false },
+	{ "\xff", false }
+};
+
+static struct {
+	unsigned length;
+	const FLAC__byte *string;
+	FLAC__bool valid;
+} VCENTRY_VALUES[] = {
+	{ 0, (const FLAC__byte*)"", true },
+	{ 1, (const FLAC__byte*)"", true },
+	{ 1, (const FLAC__byte*)"\x01", true },
+	{ 1, (const FLAC__byte*)"\x7f", true },
+	{ 1, (const FLAC__byte*)"\x80", false },
+	{ 1, (const FLAC__byte*)"\x81", false },
+	{ 1, (const FLAC__byte*)"\xc0", false },
+	{ 1, (const FLAC__byte*)"\xe0", false },
+	{ 1, (const FLAC__byte*)"\xf0", false },
+	{ 2, (const FLAC__byte*)"\xc0\x41", false },
+	{ 2, (const FLAC__byte*)"\xc1\x41", false },
+	{ 2, (const FLAC__byte*)"\xc0\x85", true },
+	{ 2, (const FLAC__byte*)"\xc1\x85", true },
+	{ 2, (const FLAC__byte*)"\xe0\x41", false },
+	{ 2, (const FLAC__byte*)"\xe1\x41", false },
+	{ 2, (const FLAC__byte*)"\xe0\x85", false },
+	{ 2, (const FLAC__byte*)"\xe1\x85", false },
+	{ 3, (const FLAC__byte*)"\xe0\x85\x41", false },
+	{ 3, (const FLAC__byte*)"\xe1\x85\x41", false },
+	{ 3, (const FLAC__byte*)"\xe0\x85\x80", true },
+	{ 3, (const FLAC__byte*)"\xe1\x85\x80", true }
+};
+
+static struct {
+	const FLAC__byte *string;
+	FLAC__bool valid;
+} VCENTRY_VALUES_NT[] = {
+	{ (FLAC__byte*)"", true },
+	{ (FLAC__byte*)"\x01", true },
+	{ (FLAC__byte*)"\x7f", true },
+	{ (FLAC__byte*)"\x80", false },
+	{ (FLAC__byte*)"\x81", false },
+	{ (FLAC__byte*)"\xc0", false },
+	{ (FLAC__byte*)"\xe0", false },
+	{ (FLAC__byte*)"\xf0", false },
+	{ (FLAC__byte*)"\xc0\x41", false },
+	{ (FLAC__byte*)"\xc1\x41", false },
+	{ (FLAC__byte*)"\xc0\x85", true },
+	{ (FLAC__byte*)"\xc1\x85", true },
+	{ (FLAC__byte*)"\xe0\x41", false },
+	{ (FLAC__byte*)"\xe1\x41", false },
+	{ (FLAC__byte*)"\xe0\x85", false },
+	{ (FLAC__byte*)"\xe1\x85", false },
+	{ (FLAC__byte*)"\xe0\x85\x41", false },
+	{ (FLAC__byte*)"\xe1\x85\x41", false },
+	{ (FLAC__byte*)"\xe0\x85\x80", true },
+	{ (FLAC__byte*)"\xe1\x85\x80", true }
+};
+
+static struct {
+	unsigned length;
+	const FLAC__byte *string;
+	FLAC__bool valid;
+} VCENTRIES[] = {
+	{ 0, (const FLAC__byte*)"", false },
+	{ 1, (const FLAC__byte*)"a", false },
+	{ 1, (const FLAC__byte*)"=", true },
+	{ 2, (const FLAC__byte*)"a=", true },
+	{ 2, (const FLAC__byte*)"\x01=", false },
+	{ 2, (const FLAC__byte*)"\x1f=", false },
+	{ 2, (const FLAC__byte*)"\x7d=", true },
+	{ 2, (const FLAC__byte*)"\x7e=", false },
+	{ 2, (const FLAC__byte *)"\xff=", false },
+	{ 3, (const FLAC__byte*)"a=\x01", true },
+	{ 3, (const FLAC__byte*)"a=\x7f", true },
+	{ 3, (const FLAC__byte*)"a=\x80", false },
+	{ 3, (const FLAC__byte*)"a=\x81", false },
+	{ 3, (const FLAC__byte*)"a=\xc0", false },
+	{ 3, (const FLAC__byte*)"a=\xe0", false },
+	{ 3, (const FLAC__byte*)"a=\xf0", false },
+	{ 4, (const FLAC__byte*)"a=\xc0\x41", false },
+	{ 4, (const FLAC__byte*)"a=\xc1\x41", false },
+	{ 4, (const FLAC__byte*)"a=\xc0\x85", true },
+	{ 4, (const FLAC__byte*)"a=\xc1\x85", true },
+	{ 4, (const FLAC__byte*)"a=\xe0\x41", false },
+	{ 4, (const FLAC__byte*)"a=\xe1\x41", false },
+	{ 4, (const FLAC__byte*)"a=\xe0\x85", false },
+	{ 4, (const FLAC__byte*)"a=\xe1\x85", false },
+	{ 5, (const FLAC__byte*)"a=\xe0\x85\x41", false },
+	{ 5, (const FLAC__byte*)"a=\xe1\x85\x41", false },
+	{ 5, (const FLAC__byte*)"a=\xe0\x85\x80", true },
+	{ 5, (const FLAC__byte*)"a=\xe1\x85\x80", true }
+};
+
+FLAC__bool test_format()
+{
+	unsigned i;
+
+	printf("\n+++ libFLAC unit test: format\n\n");
+
+	for(i = 0; i < sizeof(SAMPLE_RATES)/sizeof(SAMPLE_RATES[0]); i++) {
+		printf("testing FLAC__format_sample_rate_is_valid(%u)... ", SAMPLE_RATES[i].rate);
+		if(FLAC__format_sample_rate_is_valid(SAMPLE_RATES[i].rate) != SAMPLE_RATES[i].valid) {
+			printf("FAILED, expected %s, got %s\n", true_false_string_[SAMPLE_RATES[i].valid], true_false_string_[!SAMPLE_RATES[i].valid]);
+			return false;
+		}
+		printf("OK\n");
+	}
+
+	for(i = 0; i < sizeof(VCENTRY_NAMES)/sizeof(VCENTRY_NAMES[0]); i++) {
+		printf("testing FLAC__format_vorbiscomment_entry_name_is_legal(\"%s\")... ", VCENTRY_NAMES[i].string);
+		if(FLAC__format_vorbiscomment_entry_name_is_legal(VCENTRY_NAMES[i].string) != VCENTRY_NAMES[i].valid) {
+			printf("FAILED, expected %s, got %s\n", true_false_string_[VCENTRY_NAMES[i].valid], true_false_string_[!VCENTRY_NAMES[i].valid]);
+			return false;
+		}
+		printf("OK\n");
+	}
+
+	for(i = 0; i < sizeof(VCENTRY_VALUES)/sizeof(VCENTRY_VALUES[0]); i++) {
+		printf("testing FLAC__format_vorbiscomment_entry_value_is_legal(\"%s\", %u)... ", VCENTRY_VALUES[i].string, VCENTRY_VALUES[i].length);
+		if(FLAC__format_vorbiscomment_entry_value_is_legal(VCENTRY_VALUES[i].string, VCENTRY_VALUES[i].length) != VCENTRY_VALUES[i].valid) {
+			printf("FAILED, expected %s, got %s\n", true_false_string_[VCENTRY_VALUES[i].valid], true_false_string_[!VCENTRY_VALUES[i].valid]);
+			return false;
+		}
+		printf("OK\n");
+	}
+
+	for(i = 0; i < sizeof(VCENTRY_VALUES_NT)/sizeof(VCENTRY_VALUES_NT[0]); i++) {
+		printf("testing FLAC__format_vorbiscomment_entry_value_is_legal(\"%s\", -1)... ", VCENTRY_VALUES_NT[i].string);
+		if(FLAC__format_vorbiscomment_entry_value_is_legal(VCENTRY_VALUES_NT[i].string, (unsigned)(-1)) != VCENTRY_VALUES_NT[i].valid) {
+			printf("FAILED, expected %s, got %s\n", true_false_string_[VCENTRY_VALUES_NT[i].valid], true_false_string_[!VCENTRY_VALUES_NT[i].valid]);
+			return false;
+		}
+		printf("OK\n");
+	}
+
+	for(i = 0; i < sizeof(VCENTRIES)/sizeof(VCENTRIES[0]); i++) {
+		printf("testing FLAC__format_vorbiscomment_entry_is_legal(\"%s\", %u)... ", VCENTRIES[i].string, VCENTRIES[i].length);
+		if(FLAC__format_vorbiscomment_entry_is_legal(VCENTRIES[i].string, VCENTRIES[i].length) != VCENTRIES[i].valid) {
+			printf("FAILED, expected %s, got %s\n", true_false_string_[VCENTRIES[i].valid], true_false_string_[!VCENTRIES[i].valid]);
+			return false;
+		}
+		printf("OK\n");
+	}
+
+	printf("\nPASSED!\n");
+	return true;
+}
diff --git a/src/test_libFLAC/format.h b/src/test_libFLAC/format.h
new file mode 100644
index 00000000..567272c5
--- /dev/null
+++ b/src/test_libFLAC/format.h
@@ -0,0 +1,26 @@
+/* test_libFLAC - Unit tester for libFLAC
+ * Copyright (C) 2004  Josh Coalson
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
+ */
+
+#ifndef FLAC__TEST_LIBFLAC_FORMAT_H
+#define FLAC__TEST_LIBFLAC_FORMAT_H
+
+#include "FLAC/ordinals.h"
+
+FLAC__bool test_format();
+
+#endif
diff --git a/src/test_libFLAC/main.c b/src/test_libFLAC/main.c
index 201da68e..74c1f6bd 100644
--- a/src/test_libFLAC/main.c
+++ b/src/test_libFLAC/main.c
@@ -19,6 +19,7 @@
 #include "bitbuffer.h"
 #include "decoders.h"
 #include "encoders.h"
+#include "format.h"
 #include "metadata.h"
 
 int main(int argc, char *argv[])
@@ -28,6 +29,9 @@ int main(int argc, char *argv[])
 	if(!test_bitbuffer())
 		return 1;
 
+	if(!test_format())
+		return 1;
+
 	if(!test_encoders())
 		return 1;
 
diff --git a/src/test_libFLAC/test_libFLAC.dsp b/src/test_libFLAC/test_libFLAC.dsp
index 6ce4c61b..18fb79dd 100644
--- a/src/test_libFLAC/test_libFLAC.dsp
+++ b/src/test_libFLAC/test_libFLAC.dsp
@@ -105,6 +105,10 @@ SOURCE=.\file_utils.c
 # End Source File
 # Begin Source File
 
+SOURCE=.\format.c
+# End Source File
+# Begin Source File
+
 SOURCE=.\main.c
 # End Source File
 # Begin Source File
@@ -145,6 +149,10 @@ SOURCE=.\file_utils.h
 # End Source File
 # Begin Source File
 
+SOURCE=.\format.h
+# End Source File
+# Begin Source File
+
 SOURCE=.\metadata.h
 # End Source File
 # Begin Source File