Merge changes I9fecff3c,I2c1eb07f

* changes:
  Fix repeated packed field merging code for non-packed data.
  Add validation when parsing enum fields.

4 files changed, 124 insertions, 34 deletions

diff --git a/src/google/protobuf/compiler/javanano/javanano_enum_field.cc b/src/google/protobuf/compiler/javanano/javanano_enum_field.cc
index eb73232..6a18834 100644
--- a/src/google/protobuf/compiler/javanano/javanano_enum_field.cc
+++ b/src/google/protobuf/compiler/javanano/javanano_enum_field.cc
@@ -71,9 +71,35 @@ void SetEnumVariables(const Params& params,
   (*variables)["tag"] = SimpleItoa(internal::WireFormat::MakeTag(descriptor));
   (*variables)["tag_size"] = SimpleItoa(
       internal::WireFormat::TagSize(descriptor->number(), descriptor->type()));
+  (*variables)["non_packed_tag"] = SimpleItoa(
+      internal::WireFormatLite::MakeTag(descriptor->number(),
+          internal::WireFormat::WireTypeForFieldType(descriptor->type())));
   (*variables)["message_name"] = descriptor->containing_type()->name();
 }
 
+void LoadEnumValues(const Params& params,
+    const EnumDescriptor* enum_descriptor, vector<string>* canonical_values) {
+  string enum_class_name = ClassName(params, enum_descriptor);
+  for (int i = 0; i < enum_descriptor->value_count(); i++) {
+    const EnumValueDescriptor* value = enum_descriptor->value(i);
+    const EnumValueDescriptor* canonical_value =
+        enum_descriptor->FindValueByNumber(value->number());
+    if (value == canonical_value) {
+      canonical_values->push_back(
+          enum_class_name + "." + RenameJavaKeywords(value->name()));
+    }
+  }
+}
+
+void PrintCaseLabels(
+    io::Printer* printer, const vector<string>& canonical_values) {
+  for (int i = 0; i < canonical_values.size(); i++) {
+    printer->Print(
+      "  case $value$:\n",
+      "value", canonical_values[i]);
+  }
+}
+
 }  // namespace
 
 // ===================================================================
@@ -82,6 +108,7 @@ EnumFieldGenerator::
 EnumFieldGenerator(const FieldDescriptor* descriptor, const Params& params)
   : FieldGenerator(params), descriptor_(descriptor) {
   SetEnumVariables(params, descriptor, &variables_);
+  LoadEnumValues(params, descriptor->enum_type(), &canonical_values_);
 }
 
 EnumFieldGenerator::~EnumFieldGenerator() {}
@@ -111,12 +138,21 @@ GenerateClearCode(io::Printer* printer) const {
 void EnumFieldGenerator::
 GenerateMergingCode(io::Printer* printer) const {
   printer->Print(variables_,
-    "this.$name$ = input.readInt32();\n");
-
+    "int value = input.readInt32();\n"
+    "switch (value) {\n");
+  PrintCaseLabels(printer, canonical_values_);
+  printer->Print(variables_,
+    "    this.$name$ = value;\n");
   if (params_.generate_has()) {
     printer->Print(variables_,
-      "has$capitalized_name$ = true;\n");
+      "    has$capitalized_name$ = true;\n");
   }
+  printer->Print(
+    "    break;\n"
+    "}\n");
+  // No default case: in case of invalid value from the wire, preserve old
+  // field value. Also we are not storing the invalid value into the unknown
+  // fields, because there is no way to get the value out.
 }
 
 void EnumFieldGenerator::
@@ -209,6 +245,7 @@ AccessorEnumFieldGenerator(const FieldDescriptor* descriptor,
     const Params& params, int has_bit_index)
   : FieldGenerator(params), descriptor_(descriptor) {
   SetEnumVariables(params, descriptor, &variables_);
+  LoadEnumValues(params, descriptor->enum_type(), &canonical_values_);
   SetBitOperationVariables("has", has_bit_index, &variables_);
 }
 
@@ -245,8 +282,17 @@ GenerateClearCode(io::Printer* printer) const {
 void AccessorEnumFieldGenerator::
 GenerateMergingCode(io::Printer* printer) const {
   printer->Print(variables_,
-    "$name$_ = input.readInt32();\n"
-    "$set_has$;\n");
+    "int value = input.readInt32();\n"
+    "switch (value) {\n");
+  PrintCaseLabels(printer, canonical_values_);
+  printer->Print(variables_,
+    "    $name$_ = value;\n"
+    "    $set_has$;\n"
+    "    break;\n"
+    "}\n");
+  // No default case: in case of invalid value from the wire, preserve old
+  // field value. Also we are not storing the invalid value into the unknown
+  // fields, because there is no way to get the value out.
 }
 
 void AccessorEnumFieldGenerator::
@@ -287,6 +333,7 @@ RepeatedEnumFieldGenerator::
 RepeatedEnumFieldGenerator(const FieldDescriptor* descriptor, const Params& params)
   : FieldGenerator(params), descriptor_(descriptor) {
   SetEnumVariables(params, descriptor, &variables_);
+  LoadEnumValues(params, descriptor->enum_type(), &canonical_values_);
 }
 
 RepeatedEnumFieldGenerator::~RepeatedEnumFieldGenerator() {}
@@ -305,46 +352,82 @@ GenerateClearCode(io::Printer* printer) const {
 
 void RepeatedEnumFieldGenerator::
 GenerateMergingCode(io::Printer* printer) const {
-  // First, figure out the length of the array, then parse.
+  // First, figure out the maximum length of the array, then parse,
+  // and finally copy the valid values to the field.
   printer->Print(variables_,
-    "int arrayLength = com.google.protobuf.nano.WireFormatNano\n"
-    "    .getRepeatedFieldArrayLength(input, $tag$);\n"
-    "int i = this.$name$ == null ? 0 : this.$name$.length;\n"
-    "int[] newArray = new int[i + arrayLength];\n"
-    "if (i != 0) {\n"
-    "  java.lang.System.arraycopy(this.$name$, 0, newArray, 0, i);\n"
-    "}\n"
-    "for (; i < newArray.length - 1; i++) {\n"
-    "  newArray[i] = input.readInt32();\n"
-    "  input.readTag();\n"
+    "int length = com.google.protobuf.nano.WireFormatNano\n"
+    "    .getRepeatedFieldArrayLength(input, $non_packed_tag$);\n"
+    "int[] validValues = new int[length];\n"
+    "int validCount = 0;\n"
+    "for (int i = 0; i < length; i++) {\n"
+    "  if (i != 0) { // tag for first value already consumed.\n"
+    "    input.readTag();\n"
+    "  }\n"
+    "  int value = input.readInt32();\n"
+    "  switch (value) {\n");
+  printer->Indent();
+  PrintCaseLabels(printer, canonical_values_);
+  printer->Outdent();
+  printer->Print(variables_,
+    "      validValues[validCount++] = value;\n"
+    "      break;\n"
+    "  }\n"
     "}\n"
-    "// Last one without readTag.\n"
-    "newArray[i] = input.readInt32();\n"
-    "this.$name$ = newArray;\n");
+    "if (validCount != 0) {\n"
+    "  int i = this.$name$ == null ? 0 : this.$name$.length;\n"
+    "  if (i == 0 && validCount == validValues.length) {\n"
+    "    this.$name$ = validValues;\n"
+    "  } else {\n"
+    "    int[] newArray = new int[i + validCount];\n"
+    "    if (i != 0) {\n"
+    "      java.lang.System.arraycopy(this.$name$, 0, newArray, 0, i);\n"
+    "    }\n"
+    "    java.lang.System.arraycopy(validValues, 0, newArray, i, validCount);\n"
+    "    this.$name$ = newArray;\n"
+    "  }\n"
+    "}\n");
 }
 
 void RepeatedEnumFieldGenerator::
 GenerateMergingCodeFromPacked(io::Printer* printer) const {
   printer->Print(variables_,
-    "int length = input.readRawVarint32();\n"
-    "int limit = input.pushLimit(length);\n"
+    "int bytes = input.readRawVarint32();\n"
+    "int limit = input.pushLimit(bytes);\n"
     "// First pass to compute array length.\n"
     "int arrayLength = 0;\n"
     "int startPos = input.getPosition();\n"
     "while (input.getBytesUntilLimit() > 0) {\n"
-    "  input.readInt32();\n"
-    "  arrayLength++;\n"
-    "}\n"
-    "input.rewindToPosition(startPos);\n"
-    "int i = this.$name$ == null ? 0 : this.$name$.length;\n"
-    "int[] newArray = new int[i + arrayLength];\n"
-    "if (i != 0) {\n"
-    "  java.lang.System.arraycopy(this.$name$, 0, newArray, 0, i);\n"
+    "  switch (input.readInt32()) {\n");
+  printer->Indent();
+  PrintCaseLabels(printer, canonical_values_);
+  printer->Outdent();
+  printer->Print(variables_,
+    "      arrayLength++;\n"
+    "      break;\n"
+    "  }\n"
     "}\n"
-    "for (; i < newArray.length; i++) {\n"
-    "  newArray[i] = input.readInt32();\n"
+    "if (arrayLength != 0) {\n"
+    "  input.rewindToPosition(startPos);\n"
+    "  int i = this.$name$ == null ? 0 : this.$name$.length;\n"
+    "  int[] newArray = new int[i + arrayLength];\n"
+    "  if (i != 0) {\n"
+    "    java.lang.System.arraycopy(this.$name$, 0, newArray, 0, i);\n"
+    "  }\n"
+    "  while (input.getBytesUntilLimit() > 0) {\n"
+    "    int value = input.readInt32();\n"
+    "    switch (value) {\n");
+  printer->Indent();
+  printer->Indent();
+  PrintCaseLabels(printer, canonical_values_);
+  printer->Outdent();
+  printer->Outdent();
+  printer->Print(variables_,
+    "        newArray[i++] = value;\n"
+    "        break;\n"
+    "    }\n"
+    "  }\n"
+    "  this.$name$ = newArray;\n"
     "}\n"
-    "this.$name$ = newArray;\n"
     "input.popLimit(limit);\n");
 }
 
diff --git a/src/google/protobuf/compiler/javanano/javanano_enum_field.h b/src/google/protobuf/compiler/javanano/javanano_enum_field.h
index c477af2..55bf635 100644
--- a/src/google/protobuf/compiler/javanano/javanano_enum_field.h
+++ b/src/google/protobuf/compiler/javanano/javanano_enum_field.h
@@ -37,6 +37,7 @@
 
 #include <map>
 #include <string>
+#include <vector>
 #include <google/protobuf/compiler/javanano/javanano_field.h>
 
 namespace google {
@@ -62,6 +63,7 @@ class EnumFieldGenerator : public FieldGenerator {
  private:
   const FieldDescriptor* descriptor_;
   map<string, string> variables_;
+  vector<string> canonical_values_;
 
   GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(EnumFieldGenerator);
 };
@@ -84,6 +86,7 @@ class AccessorEnumFieldGenerator : public FieldGenerator {
  private:
   const FieldDescriptor* descriptor_;
   map<string, string> variables_;
+  vector<string> canonical_values_;
 
   GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(AccessorEnumFieldGenerator);
 };
@@ -109,6 +112,7 @@ class RepeatedEnumFieldGenerator : public FieldGenerator {
 
   const FieldDescriptor* descriptor_;
   map<string, string> variables_;
+  vector<string> canonical_values_;
 
   GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(RepeatedEnumFieldGenerator);
 };
diff --git a/src/google/protobuf/compiler/javanano/javanano_helpers.cc b/src/google/protobuf/compiler/javanano/javanano_helpers.cc
index b3bedcb..ede4c6f 100644
--- a/src/google/protobuf/compiler/javanano/javanano_helpers.cc
+++ b/src/google/protobuf/compiler/javanano/javanano_helpers.cc
@@ -412,7 +412,7 @@ string DefaultValue(const Params& params, const FieldDescriptor* field) {
 
     case FieldDescriptor::CPPTYPE_ENUM:
       return ClassName(params, field->enum_type()) + "." +
-             field->default_value_enum()->name();
+             RenameJavaKeywords(field->default_value_enum()->name());
 
     case FieldDescriptor::CPPTYPE_MESSAGE:
       return "null";
diff --git a/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc b/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc
index 9898aaf..e044e89 100644
--- a/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc
+++ b/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc
@@ -261,6 +261,9 @@ void SetPrimitiveVariables(const FieldDescriptor* descriptor, const Params param
   (*variables)["tag"] = SimpleItoa(WireFormat::MakeTag(descriptor));
   (*variables)["tag_size"] = SimpleItoa(
       WireFormat::TagSize(descriptor->number(), descriptor->type()));
+  (*variables)["non_packed_tag"] = SimpleItoa(
+      internal::WireFormatLite::MakeTag(descriptor->number(),
+          internal::WireFormat::WireTypeForFieldType(descriptor->type())));
   int fixed_size = FixedSize(descriptor->type());
   if (fixed_size != -1) {
     (*variables)["fixed_size"] = SimpleItoa(fixed_size);
@@ -748,7 +751,7 @@ GenerateMergingCode(io::Printer* printer) const {
   // First, figure out the length of the array, then parse.
   printer->Print(variables_,
     "int arrayLength = com.google.protobuf.nano.WireFormatNano\n"
-    "    .getRepeatedFieldArrayLength(input, $tag$);\n"
+    "    .getRepeatedFieldArrayLength(input, $non_packed_tag$);\n"
     "int i = this.$name$ == null ? 0 : this.$name$.length;\n");
 
   if (GetJavaType(descriptor_) == JAVATYPE_BYTES) {