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.

1 files changed, 5 insertions, 32 deletions

diff --git a/objectivec/GPBCodedOutputStream.m b/objectivec/GPBCodedOutputStream.m
index 63ba8068..251a159c 100644
--- a/objectivec/GPBCodedOutputStream.m
+++ b/objectivec/GPBCodedOutputStream.m
@@ -144,22 +144,6 @@ static void GPBWriteRawLittleEndian64(GPBOutputBufferState *state,
   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 @@ static void GPBWriteRawLittleEndian64(GPBOutputBufferState *state,
 }
 
 - (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 GPBComputeBoolSizeNoTag(BOOL value) {
 }
 
 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;
 }