commit 1a6c1d092df6431c86b74a059214ab76942b8b33
author Thomas Van Lenten <thomasvl@google.com> Mon Aug 08 18:02:43 2016 -0400
committer Thomas Van Lenten <thomasvl@google.com> Tue Aug 09 10:37:16 2016 -0400
tree 8a2d418467e7bcf2985243f90aa1900441f70c56
parent 237f321e338b50503eb38728afa6ad29bea6076a

Never use strlen on utf8 runs so null characters work.

Fixes https://github.com/google/protobuf/issues/1933

Add a new test that forces strings into two different implementations from the
NSString class cluster to help confirm we're exercising both paths by which
CodedOutputStream will extract data from an NSString.

Move the old +load test (that was flawed because the behavior really depends on
the type of string from the NSString class cluster); into a unittest that
targets the specific case we're adding a behavior confirmation on.

As a bonus, improve the TextFormat generation of string characters < 0x20.

objectivec/GPBCodedOutputStream.m

diff --git a/objectivec/GPBCodedOutputStream.m b/objectivec/GPBCodedOutputStream.m
index 63ba806..251a159 100644
--- a/objectivec/GPBCodedOutputStream.m
+++ b/objectivec/GPBCodedOutputStream.m
@@ -144,22 +144,6 @@
   GPBWriteRawByte(state, (int32_t)(value >> 56) & 0xFF);
 }
 
-#if defined(DEBUG) && DEBUG && !defined(NS_BLOCK_ASSERTIONS)
-+ (void)load {
-  // This test exists to verify that CFStrings with embedded NULLs will work
-  // for us. If this Assert fails, all code below that depends on
-  // CFStringGetCStringPtr will NOT work properly on strings that contain
-  // embedded NULLs, and we do get that in some protobufs.
-  // Note that this will not be compiled in release.
-  // We didn't feel that just keeping it in a unit test was sufficient because
-  // the Protobuf unit tests are only run when somebody is actually working
-  // on protobufs.
-  CFStringRef zeroTest = CFSTR("Test\0String");
-  const char *cString = CFStringGetCStringPtr(zeroTest, kCFStringEncodingUTF8);
-  NSAssert(cString == NULL, @"Serious Error");
-}
-#endif  // DEBUG && !defined(NS_BLOCK_ASSERTIONS)
-
 - (void)dealloc {
   [self flush];
   [state_.output close];
@@ -282,19 +266,15 @@
 }
 
 - (void)writeStringNoTag:(const NSString *)value {
-  // If you are concerned about embedded NULLs see the test in
-  // +load above.
-  const char *quickString =
-      CFStringGetCStringPtr((CFStringRef)value, kCFStringEncodingUTF8);
-  size_t length = (quickString != NULL)
-                      ? strlen(quickString)
-                      : [value lengthOfBytesUsingEncoding:NSUTF8StringEncoding];
+  size_t length = [value lengthOfBytesUsingEncoding:NSUTF8StringEncoding];
   GPBWriteRawVarint32(&state_, (int32_t)length);
-
   if (length == 0) {
     return;
   }
 
+  const char *quickString =
+      CFStringGetCStringPtr((CFStringRef)value, kCFStringEncodingUTF8);
+
   // Fast path: Most strings are short, if the buffer already has space,
   // add to it directly.
   NSUInteger bufferBytesLeft = state_.size - state_.position;
@@ -1038,14 +1018,7 @@
 }
 
 size_t GPBComputeStringSizeNoTag(NSString *value) {
-  // If you are concerned about embedded NULLs see the test in
-  // +load above.
-  const char *quickString =
-      CFStringGetCStringPtr((CFStringRef)value, kCFStringEncodingUTF8);
-  NSUInteger length =
-      (quickString != NULL)
-          ? strlen(quickString)
-          : [value lengthOfBytesUsingEncoding:NSUTF8StringEncoding];
+  NSUInteger length = [value lengthOfBytesUsingEncoding:NSUTF8StringEncoding];
   return GPBComputeRawVarint32SizeForInteger(length) + length;
 }