Merge pull request #1541 from haberman/conformancestrict

Make conformance tests more strict about the failure list.

12 files changed, 155 insertions, 163 deletions

diff --git a/conformance/Makefile.am b/conformance/Makefile.am
index 31a9e408..5538cc75 100644
--- a/conformance/Makefile.am
+++ b/conformance/Makefile.am
@@ -268,10 +268,10 @@ test_ruby: protoc_middleman conformance-test-runner $(other_language_protoc_outp
 # These depend on library paths being properly set up.  The easiest way to
 # run them is to just use "tox" from the python dir.
 test_python: protoc_middleman conformance-test-runner
-	./conformance-test-runner --failure_list failure_list_python.txt $(CONFORMANCE_PYTHON_EXTRA_FAILURES) ./conformance_python.py
+	./conformance-test-runner --failure_list failure_list_python.txt ./conformance_python.py
 
 test_python_cpp: protoc_middleman conformance-test-runner
-	./conformance-test-runner --failure_list failure_list_python_cpp.txt $(CONFORMANCE_PYTHON_EXTRA_FAILURES) ./conformance_python.py
+	./conformance-test-runner --failure_list failure_list_python_cpp.txt ./conformance_python.py
 
 if OBJC_CONFORMANCE_TEST
 
diff --git a/conformance/conformance_test.cc b/conformance/conformance_test.cc
index fc0605bf..59a61e51 100644
--- a/conformance/conformance_test.cc
+++ b/conformance/conformance_test.cc
@@ -30,6 +30,7 @@
 
 #include <stdarg.h>
 #include <string>
+#include <fstream>
 
 #include "conformance.pb.h"
 #include "conformance_test.h"
@@ -575,24 +576,41 @@ void ConformanceTestSuite::TestPrematureEOFForType(FieldDescriptor::Type type) {
   }
 }
 
-void ConformanceTestSuite::SetFailureList(const vector<string>& failure_list) {
+void ConformanceTestSuite::SetFailureList(const string& filename,
+                                          const vector<string>& failure_list) {
+  failure_list_filename_ = filename;
   expected_to_fail_.clear();
   std::copy(failure_list.begin(), failure_list.end(),
             std::inserter(expected_to_fail_, expected_to_fail_.end()));
 }
 
 bool ConformanceTestSuite::CheckSetEmpty(const set<string>& set_to_check,
-                                         const char* msg) {
+                                         const std::string& write_to_file,
+                                         const std::string& msg) {
   if (set_to_check.empty()) {
     return true;
   } else {
     StringAppendF(&output_, "\n");
-    StringAppendF(&output_, "%s:\n", msg);
+    StringAppendF(&output_, "%s\n\n", msg.c_str());
     for (set<string>::const_iterator iter = set_to_check.begin();
          iter != set_to_check.end(); ++iter) {
       StringAppendF(&output_, "  %s\n", iter->c_str());
     }
     StringAppendF(&output_, "\n");
+
+    if (!write_to_file.empty()) {
+      std::ofstream os(write_to_file);
+      if (os) {
+        for (set<string>::const_iterator iter = set_to_check.begin();
+             iter != set_to_check.end(); ++iter) {
+          os << *iter << "\n";
+        }
+      } else {
+        StringAppendF(&output_, "Failed to open file: %s\n",
+                      write_to_file.c_str());
+      }
+    }
+
     return false;
   }
 }
@@ -1965,27 +1983,34 @@ bool ConformanceTestSuite::RunSuite(ConformanceTestRunner* runner,
       )");
 
   bool ok = true;
-  if (!CheckSetEmpty(expected_to_fail_,
+  if (!CheckSetEmpty(expected_to_fail_, "nonexistent_tests.txt",
                      "These tests were listed in the failure list, but they "
-                     "don't exist.  Remove them from the failure list")) {
+                     "don't exist.  Remove them from the failure list by "
+                     "running:\n"
+                     "  ./update_failure_list.py " + failure_list_filename_ +
+                     " --remove nonexistent_tests.txt")) {
     ok = false;
   }
-  if (!CheckSetEmpty(unexpected_failing_tests_,
+  if (!CheckSetEmpty(unexpected_failing_tests_, "failing_tests.txt",
                      "These tests failed.  If they can't be fixed right now, "
                      "you can add them to the failure list so the overall "
-                     "suite can succeed")) {
+                     "suite can succeed.  Add them to the failure list by "
+                     "running:\n"
+                     "  ./update_failure_list.py " + failure_list_filename_ +
+                     " --add failing_tests.txt")) {
+    ok = false;
+  }
+  if (!CheckSetEmpty(unexpected_succeeding_tests_, "succeeding_tests.txt",
+                     "These tests succeeded, even though they were listed in "
+                     "the failure list.  Remove them from the failure list "
+                     "by running:\n"
+                     "  ./update_failure_list.py " + failure_list_filename_ +
+                     " --remove succeeding_tests.txt")) {
     ok = false;
   }
-
-  // Sometimes the testee may be fixed before we update the failure list (e.g.,
-  // the testee is from a different component). We warn about this case but
-  // don't consider it an overall test failure.
-  CheckSetEmpty(unexpected_succeeding_tests_,
-                "These tests succeeded, even though they were listed in "
-                "the failure list.  Remove them from the failure list");
 
   if (verbose_) {
-    CheckSetEmpty(skipped_,
+    CheckSetEmpty(skipped_, "",
                   "These tests were skipped (probably because support for some "
                   "features is not implemented)");
   }
diff --git a/conformance/conformance_test.h b/conformance/conformance_test.h
index 75fc97bc..c9c5213c 100644
--- a/conformance/conformance_test.h
+++ b/conformance/conformance_test.h
@@ -98,7 +98,11 @@ class ConformanceTestSuite {
   // Sets the list of tests that are expected to fail when RunSuite() is called.
   // RunSuite() will fail unless the set of failing tests is exactly the same
   // as this list.
-  void SetFailureList(const std::vector<std::string>& failure_list);
+  //
+  // The filename here is *only* used to create/format useful error messages for
+  // how to update the failure list.  We do NOT read this file at all.
+  void SetFailureList(const std::string& filename,
+                      const std::vector<std::string>& failure_list);
 
   // Run all the conformance tests against the given test runner.
   // Test output will be stored in "output".
@@ -143,12 +147,14 @@ class ConformanceTestSuite {
   void ExpectHardParseFailureForProto(const std::string& proto,
                                       const std::string& test_name);
   void TestPrematureEOFForType(google::protobuf::FieldDescriptor::Type type);
-  bool CheckSetEmpty(const set<string>& set_to_check, const char* msg);
+  bool CheckSetEmpty(const set<string>& set_to_check,
+                     const std::string& write_to_file, const std::string& msg);
   ConformanceTestRunner* runner_;
   int successes_;
   int expected_failures_;
   bool verbose_;
   std::string output_;
+  std::string failure_list_filename_;
 
   // The set of test names that are expected to fail in this run, but haven't
   // failed yet.
diff --git a/conformance/conformance_test_runner.cc b/conformance/conformance_test_runner.cc
index 376a60b9..d6b1175c 100644
--- a/conformance/conformance_test_runner.cc
+++ b/conformance/conformance_test_runner.cc
@@ -280,11 +280,13 @@ int main(int argc, char *argv[]) {
   char *program;
   google::protobuf::ConformanceTestSuite suite;
 
+  string failure_list_filename;
   vector<string> failure_list;
 
   for (int arg = 1; arg < argc; ++arg) {
     if (strcmp(argv[arg], "--failure_list") == 0) {
       if (++arg == argc) UsageError();
+      failure_list_filename = argv[arg];
       ParseFailureList(argv[arg], &failure_list);
     } else if (strcmp(argv[arg], "--verbose") == 0) {
       suite.SetVerbose(true);
@@ -300,7 +302,7 @@ int main(int argc, char *argv[]) {
     }
   }
 
-  suite.SetFailureList(failure_list);
+  suite.SetFailureList(failure_list_filename, failure_list);
   ForkPipeRunner runner(program);
 
   std::string output;
diff --git a/conformance/failure_list_cpp.txt b/conformance/failure_list_cpp.txt
index 2ddf831c..839e5210 100644
--- a/conformance/failure_list_cpp.txt
+++ b/conformance/failure_list_cpp.txt
@@ -12,21 +12,11 @@ FieldMaskPathsDontRoundTrip.JsonOutput
 FieldMaskTooManyUnderscore.JsonOutput
 JsonInput.AnyUnorderedTypeTag.JsonOutput
 JsonInput.AnyUnorderedTypeTag.ProtobufOutput
-JsonInput.AnyWithValueForInteger.JsonOutput
-JsonInput.AnyWithValueForInteger.ProtobufOutput
-JsonInput.AnyWithValueForJsonObject.JsonOutput
-JsonInput.AnyWithValueForJsonObject.ProtobufOutput
 JsonInput.BoolFieldDoubleQuotedFalse
 JsonInput.BoolFieldDoubleQuotedTrue
-JsonInput.BoolFieldIntegerOne
-JsonInput.BoolFieldIntegerZero
-JsonInput.BytesFieldInvalidBase64Characters
 JsonInput.BytesFieldNoPadding
 JsonInput.DoubleFieldTooSmall
 JsonInput.DurationHasZeroFractionalDigit.Validator
-JsonInput.DurationJsonInputTooLarge
-JsonInput.DurationJsonInputTooSmall
-JsonInput.DurationMissingS
 JsonInput.EnumFieldUnknownValue.Validator
 JsonInput.FieldMaskInvalidCharacter
 JsonInput.FieldNameDuplicate
@@ -36,58 +26,12 @@ JsonInput.FieldNameInLowerCamelCase.Validator
 JsonInput.FieldNameInSnakeCase.JsonOutput
 JsonInput.FieldNameInSnakeCase.ProtobufOutput
 JsonInput.FieldNameNotQuoted
-JsonInput.FloatFieldTooLarge
-JsonInput.FloatFieldTooSmall
-JsonInput.Int32FieldLeadingSpace
-JsonInput.Int32FieldLeadingZero
-JsonInput.Int32FieldMinFloatValue.JsonOutput
-JsonInput.Int32FieldMinFloatValue.ProtobufOutput
-JsonInput.Int32FieldMinValue.JsonOutput
-JsonInput.Int32FieldMinValue.ProtobufOutput
-JsonInput.Int32FieldNegativeWithLeadingZero
-JsonInput.Int32FieldNotInteger
-JsonInput.Int32FieldNotNumber
-JsonInput.Int32FieldTooLarge
-JsonInput.Int32FieldTooSmall
-JsonInput.Int32FieldTrailingSpace
-JsonInput.Int64FieldNotInteger
-JsonInput.Int64FieldNotNumber
-JsonInput.Int64FieldTooLarge
-JsonInput.Int64FieldTooSmall
 JsonInput.MapFieldValueIsNull
-JsonInput.OneofFieldDuplicate
 JsonInput.RepeatedFieldMessageElementIsNull
 JsonInput.RepeatedFieldPrimitiveElementIsNull
 JsonInput.RepeatedFieldTrailingComma
-JsonInput.RepeatedFieldWrongElementTypeExpectingIntegersGotBool
-JsonInput.RepeatedFieldWrongElementTypeExpectingIntegersGotMessage
-JsonInput.RepeatedFieldWrongElementTypeExpectingIntegersGotString
-JsonInput.RepeatedFieldWrongElementTypeExpectingMessagesGotBool
-JsonInput.RepeatedFieldWrongElementTypeExpectingMessagesGotInt
-JsonInput.RepeatedFieldWrongElementTypeExpectingMessagesGotString
-JsonInput.RepeatedFieldWrongElementTypeExpectingStringsGotBool
-JsonInput.RepeatedFieldWrongElementTypeExpectingStringsGotInt
-JsonInput.RepeatedFieldWrongElementTypeExpectingStringsGotMessage
-JsonInput.StringFieldNotAString
-JsonInput.StringFieldSurrogateInWrongOrder
-JsonInput.StringFieldSurrogatePair.JsonOutput
-JsonInput.StringFieldSurrogatePair.ProtobufOutput
-JsonInput.StringFieldUnpairedHighSurrogate
-JsonInput.StringFieldUnpairedLowSurrogate
 JsonInput.StringFieldUppercaseEscapeLetter
-JsonInput.TimestampJsonInputLowercaseT
-JsonInput.TimestampJsonInputLowercaseZ
-JsonInput.TimestampJsonInputMissingT
-JsonInput.TimestampJsonInputMissingZ
-JsonInput.TimestampJsonInputTooLarge
-JsonInput.TimestampJsonInputTooSmall
 JsonInput.TrailingCommaInAnObject
-JsonInput.Uint32FieldNotInteger
-JsonInput.Uint32FieldNotNumber
-JsonInput.Uint32FieldTooLarge
-JsonInput.Uint64FieldNotInteger
-JsonInput.Uint64FieldNotNumber
-JsonInput.Uint64FieldTooLarge
 JsonInput.WrapperTypesWithNullValue.JsonOutput
 JsonInput.WrapperTypesWithNullValue.ProtobufOutput
 ProtobufInput.PrematureEofBeforeKnownRepeatedValue.MESSAGE
@@ -102,5 +46,3 @@ ProtobufInput.PrematureEofInPackedField.SINT64
 ProtobufInput.PrematureEofInPackedField.UINT32
 ProtobufInput.PrematureEofInPackedField.UINT64
 ProtobufInput.PrematureEofInsideKnownRepeatedValue.MESSAGE
-TimestampProtoInputTooLarge.JsonOutput
-TimestampProtoInputTooSmall.JsonOutput
diff --git a/conformance/failure_list_csharp.txt b/conformance/failure_list_csharp.txt
index a46cee47..e7de4b96 100644
--- a/conformance/failure_list_csharp.txt
+++ b/conformance/failure_list_csharp.txt
@@ -1,16 +1,11 @@
-JsonInput.AnyWithValueForInteger.JsonOutput
-JsonInput.AnyWithValueForJsonObject.JsonOutput
 JsonInput.FieldNameInLowerCamelCase.Validator
 JsonInput.FieldNameInSnakeCase.JsonOutput
 JsonInput.FieldNameInSnakeCase.ProtobufOutput
 JsonInput.FieldNameWithMixedCases.JsonOutput
 JsonInput.FieldNameWithMixedCases.ProtobufOutput
 JsonInput.FieldNameWithMixedCases.Validator
-JsonInput.Int32FieldMinFloatValue.JsonOutput
-JsonInput.Int32FieldMinValue.JsonOutput
 JsonInput.Int64FieldMaxValueNotQuoted.JsonOutput
 JsonInput.Int64FieldMaxValueNotQuoted.ProtobufOutput
 JsonInput.OriginalProtoFieldName.JsonOutput
-JsonInput.StringFieldSurrogatePair.JsonOutput
 JsonInput.Uint64FieldMaxValueNotQuoted.JsonOutput
 JsonInput.Uint64FieldMaxValueNotQuoted.ProtobufOutput
diff --git a/conformance/failure_list_java.txt b/conformance/failure_list_java.txt
index 552c0cc9..99232ad2 100644
--- a/conformance/failure_list_java.txt
+++ b/conformance/failure_list_java.txt
@@ -8,8 +8,6 @@ FieldMaskNumbersDontRoundTrip.JsonOutput
 FieldMaskPathsDontRoundTrip.JsonOutput
 FieldMaskTooManyUnderscore.JsonOutput
 JsonInput.AnyWithFieldMask.ProtobufOutput
-JsonInput.AnyWithValueForInteger.JsonOutput
-JsonInput.AnyWithValueForJsonObject.JsonOutput
 JsonInput.BoolFieldAllCapitalFalse
 JsonInput.BoolFieldAllCapitalTrue
 JsonInput.BoolFieldCamelCaseFalse
@@ -30,8 +28,6 @@ JsonInput.FloatFieldInfinityNotQuoted
 JsonInput.FloatFieldNanNotQuoted
 JsonInput.FloatFieldNegativeInfinityNotQuoted
 JsonInput.Int32FieldLeadingZero
-JsonInput.Int32FieldMinFloatValue.JsonOutput
-JsonInput.Int32FieldMinValue.JsonOutput
 JsonInput.Int32FieldNegativeWithLeadingZero
 JsonInput.Int32FieldPlusSign
 JsonInput.Int32MapFieldKeyNotQuoted
diff --git a/conformance/failure_list_python.txt b/conformance/failure_list_python.txt
index d2e52637..550a043f 100644
--- a/conformance/failure_list_python.txt
+++ b/conformance/failure_list_python.txt
@@ -3,26 +3,7 @@ DurationProtoInputTooSmall.JsonOutput
 FieldMaskNumbersDontRoundTrip.JsonOutput
 FieldMaskPathsDontRoundTrip.JsonOutput
 FieldMaskTooManyUnderscore.JsonOutput
-JsonInput.Any.JsonOutput
-JsonInput.Any.ProtobufOutput
-JsonInput.AnyNested.JsonOutput
-JsonInput.AnyNested.ProtobufOutput
-JsonInput.AnyUnorderedTypeTag.JsonOutput
-JsonInput.AnyUnorderedTypeTag.ProtobufOutput
-JsonInput.AnyWithDuration.JsonOutput
-JsonInput.AnyWithDuration.ProtobufOutput
-JsonInput.AnyWithFieldMask.JsonOutput
 JsonInput.AnyWithFieldMask.ProtobufOutput
-JsonInput.AnyWithInt32ValueWrapper.JsonOutput
-JsonInput.AnyWithInt32ValueWrapper.ProtobufOutput
-JsonInput.AnyWithStruct.JsonOutput
-JsonInput.AnyWithStruct.ProtobufOutput
-JsonInput.AnyWithTimestamp.JsonOutput
-JsonInput.AnyWithTimestamp.ProtobufOutput
-JsonInput.AnyWithValueForInteger.JsonOutput
-JsonInput.AnyWithValueForInteger.ProtobufOutput
-JsonInput.AnyWithValueForJsonObject.JsonOutput
-JsonInput.AnyWithValueForJsonObject.ProtobufOutput
 JsonInput.BytesFieldInvalidBase64Characters
 JsonInput.DoubleFieldInfinityNotQuoted
 JsonInput.DoubleFieldNanNotQuoted
@@ -54,32 +35,13 @@ JsonInput.Int32FieldMaxFloatValue.JsonOutput
 JsonInput.Int32FieldMaxFloatValue.ProtobufOutput
 JsonInput.Int32FieldMinFloatValue.JsonOutput
 JsonInput.Int32FieldMinFloatValue.ProtobufOutput
-JsonInput.Int32FieldMinValue.JsonOutput
 JsonInput.OriginalProtoFieldName.JsonOutput
 JsonInput.OriginalProtoFieldName.ProtobufOutput
-JsonInput.RepeatedFieldMessageElementIsNull
-JsonInput.RepeatedFieldPrimitiveElementIsNull
 JsonInput.RepeatedFieldWrongElementTypeExpectingIntegersGotBool
-JsonInput.StringFieldSurrogatePair.JsonOutput
-JsonInput.StringFieldUnpairedLowSurrogate
-JsonInput.Struct.JsonOutput
-JsonInput.Struct.ProtobufOutput
 JsonInput.TimestampJsonInputLowercaseT
 JsonInput.Uint32FieldMaxFloatValue.JsonOutput
 JsonInput.Uint32FieldMaxFloatValue.ProtobufOutput
-JsonInput.ValueAcceptBool.JsonOutput
-JsonInput.ValueAcceptBool.ProtobufOutput
-JsonInput.ValueAcceptFloat.JsonOutput
-JsonInput.ValueAcceptFloat.ProtobufOutput
-JsonInput.ValueAcceptInteger.JsonOutput
-JsonInput.ValueAcceptInteger.ProtobufOutput
-JsonInput.ValueAcceptList.JsonOutput
-JsonInput.ValueAcceptList.ProtobufOutput
 JsonInput.ValueAcceptNull.JsonOutput
 JsonInput.ValueAcceptNull.ProtobufOutput
-JsonInput.ValueAcceptObject.JsonOutput
-JsonInput.ValueAcceptObject.ProtobufOutput
-JsonInput.ValueAcceptString.JsonOutput
-JsonInput.ValueAcceptString.ProtobufOutput
 TimestampProtoInputTooLarge.JsonOutput
 TimestampProtoInputTooSmall.JsonOutput
diff --git a/conformance/failure_list_python_cpp.txt b/conformance/failure_list_python_cpp.txt
index 7b5e45f9..1eb916ab 100644
--- a/conformance/failure_list_python_cpp.txt
+++ b/conformance/failure_list_python_cpp.txt
@@ -12,26 +12,7 @@ DurationProtoInputTooSmall.JsonOutput
 FieldMaskNumbersDontRoundTrip.JsonOutput
 FieldMaskPathsDontRoundTrip.JsonOutput
 FieldMaskTooManyUnderscore.JsonOutput
-JsonInput.Any.JsonOutput
-JsonInput.Any.ProtobufOutput
-JsonInput.AnyNested.JsonOutput
-JsonInput.AnyNested.ProtobufOutput
-JsonInput.AnyUnorderedTypeTag.JsonOutput
-JsonInput.AnyUnorderedTypeTag.ProtobufOutput
-JsonInput.AnyWithDuration.JsonOutput
-JsonInput.AnyWithDuration.ProtobufOutput
-JsonInput.AnyWithFieldMask.JsonOutput
 JsonInput.AnyWithFieldMask.ProtobufOutput
-JsonInput.AnyWithInt32ValueWrapper.JsonOutput
-JsonInput.AnyWithInt32ValueWrapper.ProtobufOutput
-JsonInput.AnyWithStruct.JsonOutput
-JsonInput.AnyWithStruct.ProtobufOutput
-JsonInput.AnyWithTimestamp.JsonOutput
-JsonInput.AnyWithTimestamp.ProtobufOutput
-JsonInput.AnyWithValueForInteger.JsonOutput
-JsonInput.AnyWithValueForInteger.ProtobufOutput
-JsonInput.AnyWithValueForJsonObject.JsonOutput
-JsonInput.AnyWithValueForJsonObject.ProtobufOutput
 JsonInput.BytesFieldInvalidBase64Characters
 JsonInput.DoubleFieldInfinityNotQuoted
 JsonInput.DoubleFieldNanNotQuoted
@@ -63,33 +44,14 @@ JsonInput.Int32FieldMaxFloatValue.JsonOutput
 JsonInput.Int32FieldMaxFloatValue.ProtobufOutput
 JsonInput.Int32FieldMinFloatValue.JsonOutput
 JsonInput.Int32FieldMinFloatValue.ProtobufOutput
-JsonInput.Int32FieldMinValue.JsonOutput
 JsonInput.OriginalProtoFieldName.JsonOutput
 JsonInput.OriginalProtoFieldName.ProtobufOutput
-JsonInput.RepeatedFieldMessageElementIsNull
-JsonInput.RepeatedFieldPrimitiveElementIsNull
 JsonInput.RepeatedFieldWrongElementTypeExpectingIntegersGotBool
-JsonInput.StringFieldSurrogatePair.JsonOutput
-JsonInput.StringFieldUnpairedLowSurrogate
-JsonInput.Struct.JsonOutput
-JsonInput.Struct.ProtobufOutput
 JsonInput.TimestampJsonInputLowercaseT
 JsonInput.Uint32FieldMaxFloatValue.JsonOutput
 JsonInput.Uint32FieldMaxFloatValue.ProtobufOutput
-JsonInput.ValueAcceptBool.JsonOutput
-JsonInput.ValueAcceptBool.ProtobufOutput
-JsonInput.ValueAcceptFloat.JsonOutput
-JsonInput.ValueAcceptFloat.ProtobufOutput
-JsonInput.ValueAcceptInteger.JsonOutput
-JsonInput.ValueAcceptInteger.ProtobufOutput
-JsonInput.ValueAcceptList.JsonOutput
-JsonInput.ValueAcceptList.ProtobufOutput
 JsonInput.ValueAcceptNull.JsonOutput
 JsonInput.ValueAcceptNull.ProtobufOutput
-JsonInput.ValueAcceptObject.JsonOutput
-JsonInput.ValueAcceptObject.ProtobufOutput
-JsonInput.ValueAcceptString.JsonOutput
-JsonInput.ValueAcceptString.ProtobufOutput
 ProtobufInput.PrematureEofInDelimitedDataForKnownNonRepeatedValue.MESSAGE
 ProtobufInput.PrematureEofInDelimitedDataForKnownRepeatedValue.MESSAGE
 ProtobufInput.PrematureEofInPackedField.BOOL
diff --git a/conformance/update_failure_list.py b/conformance/update_failure_list.py
new file mode 100755
index 00000000..69f210e3
--- /dev/null
+++ b/conformance/update_failure_list.py
@@ -0,0 +1,73 @@
+#!/usr/bin/env python
+# Protocol Buffers - Google's data interchange format
+# Copyright 2008 Google Inc.  All rights reserved.
+# https://developers.google.com/protocol-buffers/
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+#     * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above
+# copyright notice, this list of conditions and the following disclaimer
+# in the documentation and/or other materials provided with the
+# distribution.
+#     * Neither the name of Google Inc. nor the names of its
+# contributors may be used to endorse or promote products derived from
+# this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+"""Script to update a failure list file to add/remove failures.
+
+This is sort of like comm(1), except it recognizes comments and ignores them.
+"""
+
+import argparse
+import fileinput
+
+parser = argparse.ArgumentParser(
+    description='Adds/removes failures from the failure list.')
+parser.add_argument('filename', type=str, help='failure list file to update')
+parser.add_argument('--add', dest='add_list', action='append')
+parser.add_argument('--remove', dest='remove_list', action='append')
+
+args = parser.parse_args()
+
+add_set = set()
+remove_set = set()
+
+for add_file in (args.add_list or []):
+  with open(add_file) as f:
+    for line in f:
+      add_set.add(line)
+
+for remove_file in (args.remove_list or []):
+  with open(remove_file) as f:
+    for line in f:
+      if line in add_set:
+        raise "Asked to both add and remove test: " + line
+      remove_set.add(line.strip())
+
+add_list = sorted(add_set, reverse=True)
+
+existing_list = file(args.filename).read()
+
+with open(args.filename, "w") as f:
+  for line in existing_list.splitlines(True):
+    test = line.split("#")[0].strip()
+    while len(add_list) > 0 and test > add_list[-1]:
+      f.write(add_list.pop())
+    if test not in remove_set:
+      f.write(line)
diff --git a/python/google/protobuf/internal/json_format_test.py b/python/google/protobuf/internal/json_format_test.py
index eec1f56f..9e32ea47 100644
--- a/python/google/protobuf/internal/json_format_test.py
+++ b/python/google/protobuf/internal/json_format_test.py
@@ -247,6 +247,27 @@ class JsonFormatTest(JsonFormatBase):
     parsed_message = json_format_proto3_pb2.TestOneof()
     self.CheckParseBack(message, parsed_message)
 
+  def testSurrogates(self):
+    # Test correct surrogate handling.
+    message = json_format_proto3_pb2.TestMessage()
+    json_format.Parse('{"stringValue": "\\uD83D\\uDE01"}', message)
+    self.assertEqual(message.string_value,
+                     b'\xF0\x9F\x98\x81'.decode("utf-8", "strict"))
+
+    # TODO: add test that UTF-8 encoded surrogate code points are rejected.
+    # UTF-8 does not allow them.
+
+    # Error case: unpaired high surrogate.
+    self.CheckError(
+        '{"stringValue": "\\uD83D"}',
+        r'Invalid \\uXXXX escape|Unpaired.*surrogate')
+
+    # Unpaired low surrogate.
+    self.CheckError(
+        '{"stringValue": "\\uDE01"}',
+        r'Invalid \\uXXXX escape|Unpaired.*surrogate')
+
+
   def testTimestampMessage(self):
     message = json_format_proto3_pb2.TestTimestamp()
     message.value.seconds = 0
diff --git a/python/google/protobuf/json_format.py b/python/google/protobuf/json_format.py
index 57aa4077..be6a9b63 100644
--- a/python/google/protobuf/json_format.py
+++ b/python/google/protobuf/json_format.py
@@ -49,6 +49,7 @@ except ImportError:
 import base64
 import json
 import math
+import re
 import six
 import sys
 
@@ -68,6 +69,9 @@ _INFINITY = 'Infinity'
 _NEG_INFINITY = '-Infinity'
 _NAN = 'NaN'
 
+_UNPAIRED_SURROGATE_PATTERN = re.compile(six.u(
+    r'[\ud800-\udbff](?![\udc00-\udfff])|(?<![\ud800-\udbff])[\udc00-\udfff]'
+))
 
 class Error(Exception):
   """Top-level module error for json_format."""
@@ -555,6 +559,10 @@ def _ConvertScalarFieldValue(value, field, require_str=False):
     if field.type == descriptor.FieldDescriptor.TYPE_BYTES:
       return base64.b64decode(value)
     else:
+      # Checking for unpaired surrogates appears to be unreliable,
+      # depending on the specific Python version, so we check manually.
+      if _UNPAIRED_SURROGATE_PATTERN.search(value):
+        raise ParseError('Unpaired surrogate')
       return value
   elif field.cpp_type == descriptor.FieldDescriptor.CPPTYPE_ENUM:
     # Convert an enum value.