diff options
25 files changed, 485 insertions, 61 deletions
diff --git a/cmake/tests.cmake b/cmake/tests.cmake index 1d3be71f..978fd0c5 100644 --- a/cmake/tests.cmake +++ b/cmake/tests.cmake @@ -17,6 +17,7 @@ add_library(gmock STATIC ${protobuf_source_dir}/gmock/src/gmock-all.cc ${protobuf_source_dir}/gmock/gtest/src/gtest-all.cc ) +target_link_libraries(gmock ${CMAKE_THREAD_LIBS_INIT}) add_library(gmock_main STATIC ${protobuf_source_dir}/gmock/src/gmock_main.cc) target_link_libraries(gmock_main gmock) diff --git a/java/core/src/main/java/com/google/protobuf/CodedInputStream.java b/java/core/src/main/java/com/google/protobuf/CodedInputStream.java index 14169dc4..239798e4 100644 --- a/java/core/src/main/java/com/google/protobuf/CodedInputStream.java +++ b/java/core/src/main/java/com/google/protobuf/CodedInputStream.java @@ -354,9 +354,9 @@ public abstract class CodedInputStream { * * <p>Set the maximum message size. In order to prevent malicious messages from exhausting memory * or causing integer overflows, {@code CodedInputStream} limits how large a message may be. The - * default limit is 64MB. You should set this limit as small as you can without harming your app's - * functionality. Note that size limits only apply when reading from an {@code InputStream}, not - * when constructed around a raw byte array (nor with {@link ByteString#newCodedInput}). + * default limit is {@code Integer.MAX_INT}. You should set this limit as small as you can without + * harming your app's functionality. Note that size limits only apply when reading from an + * {@code InputStream}, not when constructed around a raw byte array. * * <p>If you want to read several messages from a single CodedInputStream, you could call {@link * #resetSizeCounter()} after each one to avoid hitting the size limit. diff --git a/java/core/src/main/java/com/google/protobuf/GeneratedMessage.java b/java/core/src/main/java/com/google/protobuf/GeneratedMessage.java index cea05794..60179e37 100644 --- a/java/core/src/main/java/com/google/protobuf/GeneratedMessage.java +++ b/java/core/src/main/java/com/google/protobuf/GeneratedMessage.java @@ -854,6 +854,7 @@ public abstract class GeneratedMessage extends AbstractMessage /** Check if a singular extension is present. */ @Override + @SuppressWarnings("unchecked") public final <Type> boolean hasExtension(final ExtensionLite<MessageType, Type> extensionLite) { Extension<MessageType, Type> extension = checkNotLite(extensionLite); @@ -863,6 +864,7 @@ public abstract class GeneratedMessage extends AbstractMessage /** Get the number of elements in a repeated extension. */ @Override + @SuppressWarnings("unchecked") public final <Type> int getExtensionCount( final ExtensionLite<MessageType, List<Type>> extensionLite) { Extension<MessageType, List<Type>> extension = checkNotLite(extensionLite); @@ -2555,6 +2557,7 @@ public abstract class GeneratedMessage extends AbstractMessage } @Override + @SuppressWarnings("unchecked") public Object get(GeneratedMessage message) { List result = new ArrayList(); for (int i = 0; i < getRepeatedCount(message); i++) { @@ -2564,6 +2567,7 @@ public abstract class GeneratedMessage extends AbstractMessage } @Override + @SuppressWarnings("unchecked") public Object get(Builder builder) { List result = new ArrayList(); for (int i = 0; i < getRepeatedCount(builder); i++) { diff --git a/java/core/src/main/java/com/google/protobuf/GeneratedMessageLite.java b/java/core/src/main/java/com/google/protobuf/GeneratedMessageLite.java index f885b01e..2d7fd334 100644 --- a/java/core/src/main/java/com/google/protobuf/GeneratedMessageLite.java +++ b/java/core/src/main/java/com/google/protobuf/GeneratedMessageLite.java @@ -220,6 +220,7 @@ public abstract class GeneratedMessageLite< } @Override + @SuppressWarnings("unchecked") public final BuilderType toBuilder() { BuilderType builder = (BuilderType) dynamicMethod(MethodToInvoke.NEW_BUILDER); builder.mergeFrom((MessageType) this); @@ -453,6 +454,7 @@ public abstract class GeneratedMessageLite< */ protected FieldSet<ExtensionDescriptor> extensions = FieldSet.newFieldSet(); + @SuppressWarnings("unchecked") protected final void mergeExtensionFields(final MessageType other) { if (extensions.isImmutable()) { extensions = extensions.clone(); diff --git a/java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java b/java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java index 2a5d8b50..fd051e75 100644 --- a/java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java +++ b/java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java @@ -866,6 +866,7 @@ public abstract class GeneratedMessageV3 extends AbstractMessage /** Check if a singular extension is present. */ @Override + @SuppressWarnings("unchecked") public final <Type> boolean hasExtension(final ExtensionLite<MessageType, Type> extensionLite) { Extension<MessageType, Type> extension = checkNotLite(extensionLite); @@ -875,6 +876,7 @@ public abstract class GeneratedMessageV3 extends AbstractMessage /** Get the number of elements in a repeated extension. */ @Override + @SuppressWarnings("unchecked") public final <Type> int getExtensionCount( final ExtensionLite<MessageType, List<Type>> extensionLite) { Extension<MessageType, List<Type>> extension = checkNotLite(extensionLite); @@ -2219,6 +2221,7 @@ public abstract class GeneratedMessageV3 extends AbstractMessage } @Override + @SuppressWarnings("unchecked") public Object get(GeneratedMessageV3 message) { List result = new ArrayList(); for (int i = 0; i < getRepeatedCount(message); i++) { @@ -2228,6 +2231,7 @@ public abstract class GeneratedMessageV3 extends AbstractMessage } @Override + @SuppressWarnings("unchecked") public Object get(Builder builder) { List result = new ArrayList(); for (int i = 0; i < getRepeatedCount(builder); i++) { diff --git a/java/core/src/main/java/com/google/protobuf/MapEntry.java b/java/core/src/main/java/com/google/protobuf/MapEntry.java index 179c3348..7e8e9aad 100644 --- a/java/core/src/main/java/com/google/protobuf/MapEntry.java +++ b/java/core/src/main/java/com/google/protobuf/MapEntry.java @@ -89,6 +89,7 @@ public final class MapEntry<K, V> extends AbstractMessage { } /** Create a MapEntry with the provided key and value. */ + @SuppressWarnings("unchecked") private MapEntry(Metadata metadata, K key, V value) { this.key = key; this.value = value; @@ -435,6 +436,7 @@ public final class MapEntry<K, V> extends AbstractMessage { } @Override + @SuppressWarnings("unchecked") public Builder<K, V> clone() { return new Builder(metadata, key, value); } diff --git a/java/core/src/main/java/com/google/protobuf/MapField.java b/java/core/src/main/java/com/google/protobuf/MapField.java index a6109f98..805defe2 100644 --- a/java/core/src/main/java/com/google/protobuf/MapField.java +++ b/java/core/src/main/java/com/google/protobuf/MapField.java @@ -100,6 +100,7 @@ public class MapField<K, V> implements MutabilityOracle { } @Override + @SuppressWarnings("unchecked") public void convertMessageToKeyAndValue(Message message, Map<K, V> map) { MapEntry<K, V> entry = (MapEntry<K, V>) message; map.put(entry.getKey(), entry.getValue()); diff --git a/java/core/src/test/java/com/google/protobuf/TestUtil.java b/java/core/src/test/java/com/google/protobuf/TestUtil.java index d4a18a22..c1bd21db 100644 --- a/java/core/src/test/java/com/google/protobuf/TestUtil.java +++ b/java/core/src/test/java/com/google/protobuf/TestUtil.java @@ -2602,6 +2602,9 @@ public final class TestUtil { case FOO_CORD: Assert.assertTrue(message.hasFooCord()); break; + case FOO_STRING_PIECE: + Assert.assertTrue(message.hasFooStringPiece()); + break; case FOO_BYTES: Assert.assertTrue(message.hasFooBytes()); break; diff --git a/java/util/src/main/java/com/google/protobuf/util/JsonFormat.java b/java/util/src/main/java/com/google/protobuf/util/JsonFormat.java index ac712c94..838700f7 100644 --- a/java/util/src/main/java/com/google/protobuf/util/JsonFormat.java +++ b/java/util/src/main/java/com/google/protobuf/util/JsonFormat.java @@ -35,6 +35,7 @@ import com.google.gson.Gson; import com.google.gson.GsonBuilder; import com.google.gson.JsonArray; import com.google.gson.JsonElement; +import com.google.gson.JsonIOException; import com.google.gson.JsonNull; import com.google.gson.JsonObject; import com.google.gson.JsonParser; @@ -1067,9 +1068,23 @@ public class JsonFormat { } void merge(Reader json, Message.Builder builder) throws IOException { - JsonReader reader = new JsonReader(json); - reader.setLenient(false); - merge(jsonParser.parse(reader), builder); + try { + JsonReader reader = new JsonReader(json); + reader.setLenient(false); + merge(jsonParser.parse(reader), builder); + } catch (InvalidProtocolBufferException e) { + throw e; + } catch (JsonIOException e) { + // Unwrap IOException. + if (e.getCause() instanceof IOException) { + throw (IOException) e.getCause(); + } else { + throw new InvalidProtocolBufferException(e.getMessage()); + } + } catch (Exception e) { + // We convert all exceptions from JSON parsing to our own exceptions. + throw new InvalidProtocolBufferException(e.getMessage()); + } } void merge(String json, Message.Builder builder) throws InvalidProtocolBufferException { diff --git a/java/util/src/main/java/com/google/protobuf/util/Timestamps.java b/java/util/src/main/java/com/google/protobuf/util/Timestamps.java index 2160e4d5..1d631a2c 100644 --- a/java/util/src/main/java/com/google/protobuf/util/Timestamps.java +++ b/java/util/src/main/java/com/google/protobuf/util/Timestamps.java @@ -44,6 +44,7 @@ import java.text.SimpleDateFormat; import java.util.Comparator; import java.util.Date; import java.util.GregorianCalendar; +import java.util.Locale; import java.util.TimeZone; /** @@ -83,7 +84,7 @@ public final class Timestamps { }; private static SimpleDateFormat createTimestampFormat() { - SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss"); + SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss", Locale.ENGLISH); GregorianCalendar calendar = new GregorianCalendar(TimeZone.getTimeZone("UTC")); // We use Proleptic Gregorian Calendar (i.e., Gregorian calendar extends // backwards to year one) for timestamp formating. @@ -386,11 +387,11 @@ public final class Timestamps { static String formatNanos(int nanos) { // Determine whether to use 3, 6, or 9 digits for the nano part. if (nanos % NANOS_PER_MILLISECOND == 0) { - return String.format("%1$03d", nanos / NANOS_PER_MILLISECOND); + return String.format(Locale.ENGLISH, "%1$03d", nanos / NANOS_PER_MILLISECOND); } else if (nanos % NANOS_PER_MICROSECOND == 0) { - return String.format("%1$06d", nanos / NANOS_PER_MICROSECOND); + return String.format(Locale.ENGLISH, "%1$06d", nanos / NANOS_PER_MICROSECOND); } else { - return String.format("%1$09d", nanos); + return String.format(Locale.ENGLISH, "%1$09d", nanos); } } } diff --git a/java/util/src/test/java/com/google/protobuf/util/JsonFormatTest.java b/java/util/src/test/java/com/google/protobuf/util/JsonFormatTest.java index dafd9bb5..de02c117 100644 --- a/java/util/src/test/java/com/google/protobuf/util/JsonFormatTest.java +++ b/java/util/src/test/java/com/google/protobuf/util/JsonFormatTest.java @@ -62,13 +62,23 @@ import com.google.protobuf.util.JsonTestProto.TestStruct; import com.google.protobuf.util.JsonTestProto.TestTimestamp; import com.google.protobuf.util.JsonTestProto.TestWrappers; import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.io.Reader; +import java.io.StringReader; import java.math.BigDecimal; import java.math.BigInteger; import java.util.HashMap; +import java.util.Locale; import java.util.Map; import junit.framework.TestCase; public class JsonFormatTest extends TestCase { + public JsonFormatTest() { + // Test that locale does not affect JsonFormat. + Locale.setDefault(Locale.forLanguageTag("hi-IN")); + } + private void setAllFields(TestAllTypes.Builder builder) { builder.setOptionalInt32(1234); builder.setOptionalInt64(1234567890123456789L); @@ -1411,4 +1421,34 @@ public class JsonFormatTest extends TestCase { // Expected. } } + + // Test that we are not leaking out JSON exceptions. + public void testJsonException() throws Exception { + InputStream throwingInputStream = new InputStream() { + public int read() throws IOException { + throw new IOException("12345"); + } + }; + InputStreamReader throwingReader = new InputStreamReader(throwingInputStream); + // When the underlying reader throws IOException, JsonFormat should forward + // through this IOException. + try { + TestAllTypes.Builder builder = TestAllTypes.newBuilder(); + JsonFormat.parser().merge(throwingReader, builder); + fail("Exception is expected."); + } catch (IOException e) { + assertEquals("12345", e.getMessage()); + } + + Reader invalidJsonReader = new StringReader("{ xxx - yyy }"); + // When the JSON parser throws parser exceptions, JsonFormat should turn + // that into InvalidProtocolBufferException. + try { + TestAllTypes.Builder builder = TestAllTypes.newBuilder(); + JsonFormat.parser().merge(invalidJsonReader, builder); + fail("Exception is expected."); + } catch (InvalidProtocolBufferException e) { + // Expected. + } + } } diff --git a/php/ext/google/protobuf/array.c b/php/ext/google/protobuf/array.c index 63bb6d0a..ab2eaf90 100644 --- a/php/ext/google/protobuf/array.c +++ b/php/ext/google/protobuf/array.c @@ -225,8 +225,17 @@ void repeated_field_push_native(RepeatedField *intern, void *value TSRMLS_DC) { zend_hash_next_index_insert(ht, (void **)value, size, NULL); } +void repeated_field_create_with_field(zend_class_entry *ce, + const upb_fielddef *field, + zval **repeated_field TSRMLS_DC) { + upb_fieldtype_t type = upb_fielddef_type(field); + const zend_class_entry *msg_ce = field_type_class(field TSRMLS_CC); + repeated_field_create_with_type(ce, type, msg_ce, repeated_field); +} + void repeated_field_create_with_type(zend_class_entry *ce, - const upb_fielddef *field, + upb_fieldtype_t type, + const zend_class_entry* msg_ce, zval **repeated_field TSRMLS_DC) { MAKE_STD_ZVAL(*repeated_field); Z_TYPE_PP(repeated_field) = IS_OBJECT; @@ -235,13 +244,8 @@ void repeated_field_create_with_type(zend_class_entry *ce, RepeatedField *intern = zend_object_store_get_object(*repeated_field TSRMLS_CC); - intern->type = upb_fielddef_type(field); - if (intern->type == UPB_TYPE_MESSAGE) { - const upb_msgdef *msg = upb_fielddef_msgsubdef(field); - zval *desc_php = get_def_obj(msg); - Descriptor *desc = zend_object_store_get_object(desc_php TSRMLS_CC); - intern->msg_ce = desc->klass; - } + intern->type = type; + intern->msg_ce = msg_ce; MAKE_STD_ZVAL(intern->array); repeated_field_array_init(intern->array, intern->type, 0 ZEND_FILE_LINE_CC); diff --git a/php/ext/google/protobuf/map.c b/php/ext/google/protobuf/map.c index ab98879d..4a3829d6 100644 --- a/php/ext/google/protobuf/map.c +++ b/php/ext/google/protobuf/map.c @@ -146,6 +146,11 @@ static zend_function_entry map_field_methods[] = { ZEND_FE_END }; +// Forward declare static functions. + +static bool map_field_write_dimension(zval *object, zval *key, + zval *value TSRMLS_DC); + // ----------------------------------------------------------------------------- // MapField creation/desctruction // ----------------------------------------------------------------------------- @@ -183,6 +188,7 @@ void map_field_init(TSRMLS_D) { map_field_handlers = PEMALLOC(zend_object_handlers); memcpy(map_field_handlers, zend_get_std_object_handlers(), sizeof(zend_object_handlers)); + map_field_handlers->write_dimension = map_field_write_dimension; map_field_handlers->get_gc = map_field_get_gc; } @@ -235,7 +241,18 @@ void map_field_free(void *object TSRMLS_DC) { efree(object); } -void map_field_create_with_type(zend_class_entry *ce, const upb_fielddef *field, +void map_field_create_with_field(zend_class_entry *ce, const upb_fielddef *field, + zval **map_field TSRMLS_DC) { + const upb_fielddef *key_field = map_field_key(field); + const upb_fielddef *value_field = map_field_value(field); + map_field_create_with_type( + ce, upb_fielddef_type(key_field), upb_fielddef_type(value_field), + field_type_class(value_field TSRMLS_CC), map_field); +} + +void map_field_create_with_type(zend_class_entry *ce, upb_fieldtype_t key_type, + upb_fieldtype_t value_type, + const zend_class_entry *msg_ce, zval **map_field TSRMLS_DC) { MAKE_STD_ZVAL(*map_field); Z_TYPE_PP(map_field) = IS_OBJECT; @@ -245,11 +262,9 @@ void map_field_create_with_type(zend_class_entry *ce, const upb_fielddef *field, Map* intern = (Map*)zend_object_store_get_object(*map_field TSRMLS_CC); - const upb_fielddef *key_field = map_field_key(field); - const upb_fielddef *value_field = map_field_value(field); - intern->key_type = upb_fielddef_type(key_field); - intern->value_type = upb_fielddef_type(value_field); - intern->msg_ce = field_type_class(value_field TSRMLS_CC); + intern->key_type = key_type; + intern->value_type = value_type; + intern->msg_ce = msg_ce; } static void map_field_free_element(void *object) { diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h index 28cea95c..2287f7e6 100644 --- a/php/ext/google/protobuf/protobuf.h +++ b/php/ext/google/protobuf/protobuf.h @@ -296,6 +296,7 @@ PHP_METHOD(Util, checkBool); PHP_METHOD(Util, checkString); PHP_METHOD(Util, checkBytes); PHP_METHOD(Util, checkMessage); +PHP_METHOD(Util, checkMapField); PHP_METHOD(Util, checkRepeatedField); // ----------------------------------------------------------------------------- @@ -349,7 +350,11 @@ const upb_fielddef* map_entry_key(const upb_msgdef* msgdef); const upb_fielddef* map_entry_value(const upb_msgdef* msgdef); zend_object_value map_field_create(zend_class_entry *ce TSRMLS_DC); -void map_field_create_with_type(zend_class_entry *ce, const upb_fielddef *field, +void map_field_create_with_field(zend_class_entry *ce, const upb_fielddef *field, + zval **map_field TSRMLS_DC); +void map_field_create_with_type(zend_class_entry *ce, upb_fieldtype_t key_type, + upb_fieldtype_t value_type, + const zend_class_entry *msg_ce, zval **map_field TSRMLS_DC); void map_field_free(void* object TSRMLS_DC); void* upb_value_memory(upb_value* v); @@ -392,9 +397,12 @@ struct RepeatedFieldIter { long position; }; -void repeated_field_create_with_type(zend_class_entry* ce, +void repeated_field_create_with_field(zend_class_entry* ce, const upb_fielddef* field, zval** repeated_field TSRMLS_DC); +void repeated_field_create_with_type(zend_class_entry* ce, upb_fieldtype_t type, + const zend_class_entry* msg_ce, + zval** repeated_field TSRMLS_DC); // Return the element at the index position from the repeated field. There is // not restriction on the type of stored elements. void *repeated_field_index_native(RepeatedField *intern, int index TSRMLS_DC); diff --git a/php/ext/google/protobuf/storage.c b/php/ext/google/protobuf/storage.c index 1b239ee3..af7c292f 100644 --- a/php/ext/google/protobuf/storage.c +++ b/php/ext/google/protobuf/storage.c @@ -517,12 +517,12 @@ void layout_init(MessageLayout* layout, void* storage, *oneof_case = ONEOF_CASE_NONE; } else if (is_map_field(field)) { zval_ptr_dtor(property_ptr); - map_field_create_with_type(map_field_type, field, property_ptr TSRMLS_CC); + map_field_create_with_field(map_field_type, field, property_ptr TSRMLS_CC); DEREF(memory, zval**) = property_ptr; } else if (upb_fielddef_label(field) == UPB_LABEL_REPEATED) { zval_ptr_dtor(property_ptr); - repeated_field_create_with_type(repeated_field_type, field, - property_ptr TSRMLS_CC); + repeated_field_create_with_field(repeated_field_type, field, + property_ptr TSRMLS_CC); DEREF(memory, zval**) = property_ptr; } else { native_slot_init(upb_fielddef_type(field), memory, property_ptr); diff --git a/php/ext/google/protobuf/type_check.c b/php/ext/google/protobuf/type_check.c index d12d0025..718682ac 100644 --- a/php/ext/google/protobuf/type_check.c +++ b/php/ext/google/protobuf/type_check.c @@ -51,6 +51,13 @@ ZEND_BEGIN_ARG_INFO_EX(arg_check_repeated, 0, 0, 2) ZEND_ARG_INFO(0, klass) ZEND_END_ARG_INFO() +ZEND_BEGIN_ARG_INFO_EX(arg_check_map, 0, 0, 3) + ZEND_ARG_INFO(1, val) + ZEND_ARG_INFO(0, key_type) + ZEND_ARG_INFO(0, value_type) + ZEND_ARG_INFO(0, klass) +ZEND_END_ARG_INFO() + static zend_function_entry util_methods[] = { PHP_ME(Util, checkInt32, arg_check_optional, ZEND_ACC_PUBLIC|ZEND_ACC_STATIC) PHP_ME(Util, checkUint32, arg_check_optional, ZEND_ACC_PUBLIC|ZEND_ACC_STATIC) @@ -63,6 +70,7 @@ static zend_function_entry util_methods[] = { PHP_ME(Util, checkString, arg_check_optional, ZEND_ACC_PUBLIC|ZEND_ACC_STATIC) PHP_ME(Util, checkBytes, arg_check_optional, ZEND_ACC_PUBLIC|ZEND_ACC_STATIC) PHP_ME(Util, checkMessage, arg_check_message, ZEND_ACC_PUBLIC|ZEND_ACC_STATIC) + PHP_ME(Util, checkMapField, arg_check_map, ZEND_ACC_PUBLIC|ZEND_ACC_STATIC) PHP_ME(Util, checkRepeatedField, arg_check_repeated, ZEND_ACC_PUBLIC|ZEND_ACC_STATIC) ZEND_FE_END @@ -411,20 +419,112 @@ PHP_METHOD(Util, checkRepeatedField) { zval* val; long type; const zend_class_entry* klass = NULL; - if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "Ol|C", &val, - repeated_field_type, &type, &klass) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "zl|C", &val, &type, + &klass) == FAILURE) { return; } - RepeatedField *intern = - (RepeatedField *)zend_object_store_get_object(val TSRMLS_CC); - if (to_fieldtype(type) != intern->type) { + if (Z_TYPE_P(val) == IS_ARRAY) { + HashTable* table = Z_ARRVAL_P(val); + HashPosition pointer; + void* memory; + zval* repeated_field; + + repeated_field_create_with_type(repeated_field_type, to_fieldtype(type), + klass, &repeated_field TSRMLS_CC); + RepeatedField* intern = + (RepeatedField*)zend_object_store_get_object(repeated_field TSRMLS_CC); + + for (zend_hash_internal_pointer_reset_ex(table, &pointer); + zend_hash_get_current_data_ex(table, (void**)&memory, &pointer) == + SUCCESS; + zend_hash_move_forward_ex(table, &pointer)) { + repeated_field_handlers->write_dimension(repeated_field, NULL, *(zval**)memory); + } + + Z_DELREF_P(repeated_field); + RETURN_ZVAL(repeated_field, 1, 0); + + } else if (Z_TYPE_P(val) == IS_OBJECT) { + if (!instanceof_function(Z_OBJCE_P(val), repeated_field_type TSRMLS_CC)) { + zend_error(E_USER_ERROR, "Given value is not an instance of %s.", + repeated_field_type->name); + return; + } + RepeatedField* intern = + (RepeatedField*)zend_object_store_get_object(val TSRMLS_CC); + if (to_fieldtype(type) != intern->type) { + zend_error(E_USER_ERROR, "Incorrect repeated field type."); + return; + } + if (klass != NULL && intern->msg_ce != klass) { + zend_error(E_USER_ERROR, + "Expect a repeated field of %s, but %s is given.", klass->name, + intern->msg_ce->name); + return; + } + RETURN_ZVAL(val, 1, 0); + } else { zend_error(E_USER_ERROR, "Incorrect repeated field type."); return; } - if (klass != NULL && intern->msg_ce != klass) { - zend_error(E_USER_ERROR, "Expect a repeated field of %s, but %s is given.", - klass->name, intern->msg_ce->name); + +} + +PHP_METHOD(Util, checkMapField) { + zval* val; + long key_type, value_type; + const zend_class_entry* klass = NULL; + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "zll|C", &val, &key_type, + &value_type, &klass) == FAILURE) { + return; + } + + if (Z_TYPE_P(val) == IS_ARRAY) { + HashTable* table = Z_ARRVAL_P(val); + HashPosition pointer; + zval key, *map_field; + void* value; + + map_field_create_with_type(map_field_type, to_fieldtype(key_type), + to_fieldtype(value_type), klass, + &map_field TSRMLS_CC); + Map* intern = + (Map*)zend_object_store_get_object(map_field TSRMLS_CC); + + for (zend_hash_internal_pointer_reset_ex(table, &pointer); + zend_hash_get_current_data_ex(table, (void**)&value, &pointer) == + SUCCESS; + zend_hash_move_forward_ex(table, &pointer)) { + zend_hash_get_current_key_zval_ex(table, &key, &pointer); + map_field_handlers->write_dimension(map_field, &key, *(zval**)value); + } + + Z_DELREF_P(map_field); + RETURN_ZVAL(map_field, 1, 0); + } else if (Z_TYPE_P(val) == IS_OBJECT) { + if (!instanceof_function(Z_OBJCE_P(val), map_field_type TSRMLS_CC)) { + zend_error(E_USER_ERROR, "Given value is not an instance of %s.", + map_field_type->name); + return; + } + Map* intern = (Map*)zend_object_store_get_object(val TSRMLS_CC); + if (to_fieldtype(key_type) != intern->key_type) { + zend_error(E_USER_ERROR, "Incorrect map field key type."); + return; + } + if (to_fieldtype(value_type) != intern->value_type) { + zend_error(E_USER_ERROR, "Incorrect map field value type."); + return; + } + if (klass != NULL && intern->msg_ce != klass) { + zend_error(E_USER_ERROR, "Expect a map field of %s, but %s is given.", + klass->name, intern->msg_ce->name); + return; + } + RETURN_ZVAL(val, 1, 0); + } else { + zend_error(E_USER_ERROR, "Incorrect map field type."); return; } } diff --git a/php/src/Google/Protobuf/Internal/GPBUtil.php b/php/src/Google/Protobuf/Internal/GPBUtil.php index ba1d2eb3..0e66ae6f 100644 --- a/php/src/Google/Protobuf/Internal/GPBUtil.php +++ b/php/src/Google/Protobuf/Internal/GPBUtil.php @@ -34,6 +34,7 @@ namespace Google\Protobuf\Internal; use Google\Protobuf\Internal\GPBType; use Google\Protobuf\Internal\RepeatedField; +use Google\Protobuf\Internal\MapField; class GPBUtil { @@ -175,19 +176,60 @@ class GPBUtil public static function checkRepeatedField(&$var, $type, $klass = null) { - if (!$var instanceof RepeatedField) { - trigger_error("Expect repeated field.", E_USER_ERROR); - } - if ($var->getType() != $type) { - trigger_error( - "Expect repeated field of different type.", - E_USER_ERROR); - } - if ($var->getType() === GPBType::MESSAGE && - $var->getClass() !== $klass) { - trigger_error( - "Expect repeated field of different message.", - E_USER_ERROR); + if (!$var instanceof RepeatedField && !is_array($var)) { + trigger_error("Expect array.", E_USER_ERROR); + } + if (is_array($var)) { + $tmp = new RepeatedField($type, $klass); + foreach ($var as $value) { + $tmp[] = $value; + } + return $tmp; + } else { + if ($var->getType() != $type) { + trigger_error( + "Expect repeated field of different type.", + E_USER_ERROR); + } + if ($var->getType() === GPBType::MESSAGE && + $var->getClass() !== $klass) { + trigger_error( + "Expect repeated field of different message.", + E_USER_ERROR); + } + return $var; + } + } + + public static function checkMapField(&$var, $key_type, $value_type, $klass = null) + { + if (!$var instanceof MapField && !is_array($var)) { + trigger_error("Expect dict.", E_USER_ERROR); + } + if (is_array($var)) { + $tmp = new MapField($key_type, $value_type, $klass); + foreach ($var as $key => $value) { + $tmp[$key] = $value; + } + return $tmp; + } else { + if ($var->getKeyType() != $key_type) { + trigger_error( + "Expect map field of key type.", + E_USER_ERROR); + } + if ($var->getValueType() != $value_type) { + trigger_error( + "Expect map field of value type.", + E_USER_ERROR); + } + if ($var->getValueType() === GPBType::MESSAGE && + $var->getValueClass() !== $klass) { + trigger_error( + "Expect map field of different value message.", + E_USER_ERROR); + } + return $var; } } diff --git a/php/src/Google/Protobuf/Internal/MapField.php b/php/src/Google/Protobuf/Internal/MapField.php index 14ee7ebe..68c10c08 100644 --- a/php/src/Google/Protobuf/Internal/MapField.php +++ b/php/src/Google/Protobuf/Internal/MapField.php @@ -204,6 +204,30 @@ class MapField implements \ArrayAccess, \IteratorAggregate, \Countable } /** + * @ignore + */ + public function getKeyType() + { + return $this->key_type; + } + + /** + * @ignore + */ + public function getValueType() + { + return $this->value_type; + } + + /** + * @ignore + */ + public function getValueClass() + { + return $this->klass; + } + + /** * Return the element at the given key. * * This will also be called for: $ele = $arr[$key] diff --git a/php/tests/generated_class_test.php b/php/tests/generated_class_test.php index 7f8567b8..4c3bca2d 100644 --- a/php/tests/generated_class_test.php +++ b/php/tests/generated_class_test.php @@ -6,6 +6,7 @@ require_once('test_base.php'); require_once('test_util.php'); use Google\Protobuf\Internal\RepeatedField; +use Google\Protobuf\Internal\MapField; use Google\Protobuf\Internal\GPBType; use Foo\TestEnum; use Foo\TestMessage; @@ -526,6 +527,26 @@ class GeneratedClassTest extends TestBase $this->assertSame($repeated_int32, $m->getRepeatedInt32()); } + public function testRepeatedFieldViaArray() + { + $m = new TestMessage(); + + $arr = array(); + $m->setRepeatedInt32($arr); + $this->assertSame(0, count($m->getRepeatedInt32())); + + $arr = array(1, 2.1, "3"); + $m->setRepeatedInt32($arr); + $this->assertTrue($m->getRepeatedInt32() instanceof RepeatedField); + $this->assertSame("Google\Protobuf\Internal\RepeatedField", + get_class($m->getRepeatedInt32())); + $this->assertSame(3, count($m->getRepeatedInt32())); + $this->assertSame(1, $m->getRepeatedInt32()[0]); + $this->assertSame(2, $m->getRepeatedInt32()[1]); + $this->assertSame(3, $m->getRepeatedInt32()[2]); + $this->assertFalse($arr instanceof RepeatedField); + } + /** * @expectedException PHPUnit_Framework_Error */ @@ -569,6 +590,80 @@ class GeneratedClassTest extends TestBase } ######################################################### + # Test map field. + ######################################################### + + public function testMapField() + { + $m = new TestMessage(); + + $map_int32_int32 = new MapField(GPBType::INT32, GPBType::INT32); + $m->setMapInt32Int32($map_int32_int32); + $this->assertSame($map_int32_int32, $m->getMapInt32Int32()); + } + + public function testMapFieldViaArray() + { + $m = new TestMessage(); + + $dict = array(); + $m->setMapInt32Int32($dict); + $this->assertSame(0, count($m->getMapInt32Int32())); + + $dict = array(5 => 5, 6.1 => 6.1, "7" => "7"); + $m->setMapInt32Int32($dict); + $this->assertTrue($m->getMapInt32Int32() instanceof MapField); + $this->assertSame(3, count($m->getMapInt32Int32())); + $this->assertSame(5, $m->getMapInt32Int32()[5]); + $this->assertSame(6, $m->getMapInt32Int32()[6]); + $this->assertSame(7, $m->getMapInt32Int32()[7]); + $this->assertFalse($dict instanceof MapField); + } + + /** + * @expectedException PHPUnit_Framework_Error + */ + public function testMapFieldWrongTypeFail() + { + $m = new TestMessage(); + $a = 1; + $m->setMapInt32Int32($a); + } + + /** + * @expectedException PHPUnit_Framework_Error + */ + public function testMapFieldWrongObjectFail() + { + $m = new TestMessage(); + $m->setMapInt32Int32($m); + } + + /** + * @expectedException PHPUnit_Framework_Error + */ + public function testMapFieldWrongRepeatedTypeFail() + { + $m = new TestMessage(); + + $map_uint32_uint32 = new MapField(GPBType::UINT32, GPBType::UINT32); + $m->setMapInt32Int32($map_uint32_uint32); + } + + /** + * @expectedException PHPUnit_Framework_Error + */ + public function testMapFieldWrongRepeatedMessageClassFail() + { + $m = new TestMessage(); + + $map_int32_message = new MapField(GPBType::INT32, + GPBType::MESSAGE, + TestMessage::class); + $m->setMapInt32Message($map_int32_message); + } + + ######################################################### # Test oneof field. ######################################################### diff --git a/src/google/protobuf/compiler/java/java_message.cc b/src/google/protobuf/compiler/java/java_message.cc index 652c5d78..3b8d7ab8 100644 --- a/src/google/protobuf/compiler/java/java_message.cc +++ b/src/google/protobuf/compiler/java/java_message.cc @@ -1453,6 +1453,7 @@ void ImmutableMessageGenerator::GenerateAnyMethods(io::Printer* printer) { "\n" "private volatile com.google.protobuf.Message cachedUnpackValue;\n" "\n" + "@java.lang.SuppressWarnings(\"unchecked\")\n" "public <T extends com.google.protobuf.Message> T unpack(\n" " java.lang.Class<T> clazz)\n" " throws com.google.protobuf.InvalidProtocolBufferException {\n" diff --git a/src/google/protobuf/compiler/java/java_message_lite.cc b/src/google/protobuf/compiler/java/java_message_lite.cc index 9720cacf..e84321bb 100644 --- a/src/google/protobuf/compiler/java/java_message_lite.cc +++ b/src/google/protobuf/compiler/java/java_message_lite.cc @@ -345,6 +345,7 @@ void ImmutableMessageLiteGenerator::Generate(io::Printer* printer) { } printer->Print( + "@java.lang.SuppressWarnings({\"unchecked\", \"fallthrough\"})\n" "protected final Object dynamicMethod(\n" " com.google.protobuf.GeneratedMessageLite.MethodToInvoke method,\n" " Object arg0, Object arg1) {\n" diff --git a/src/google/protobuf/compiler/php/php_generator.cc b/src/google/protobuf/compiler/php/php_generator.cc index ec9a2365..7f35d712 100644 --- a/src/google/protobuf/compiler/php/php_generator.cc +++ b/src/google/protobuf/compiler/php/php_generator.cc @@ -439,9 +439,31 @@ void GenerateFieldAccessor(const FieldDescriptor* field, bool is_descriptor, // Type check. if (field->is_map()) { + const Descriptor* map_entry = field->message_type(); + const FieldDescriptor* key = map_entry->FindFieldByName("key"); + const FieldDescriptor* value = map_entry->FindFieldByName("value"); + printer->Print( + "$arr = GPBUtil::checkMapField($var, " + "\\Google\\Protobuf\\Internal\\GPBType::^key_type^, " + "\\Google\\Protobuf\\Internal\\GPBType::^value_type^", + "key_type", ToUpper(key->type_name()), + "value_type", ToUpper(value->type_name())); + if (value->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) { + printer->Print( + ", \\^class_name^);\n", + "class_name", + MessageName(value->message_type(), is_descriptor) + "::class"); + } else if (value->cpp_type() == FieldDescriptor::CPPTYPE_ENUM) { + printer->Print( + ", ^class_name^);\n", + "class_name", + EnumName(value->enum_type(), is_descriptor) + "::class"); + } else { + printer->Print(");\n"); + } } else if (field->is_repeated()) { printer->Print( - "GPBUtil::checkRepeatedField($var, " + "$arr = GPBUtil::checkRepeatedField($var, " "\\Google\\Protobuf\\Internal\\GPBType::^type^", "type", ToUpper(field->type_name())); if (field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) { @@ -480,6 +502,10 @@ void GenerateFieldAccessor(const FieldDescriptor* field, bool is_descriptor, printer->Print( "$this->writeOneof(^number^, $var);\n", "number", IntToString(field->number())); + } else if (field->is_repeated()) { + printer->Print( + "$this->^name^ = $arr;\n", + "name", field->name()); } else { printer->Print( "$this->^name^ = $var;\n", diff --git a/src/google/protobuf/util/internal/protostream_objectwriter_test.cc b/src/google/protobuf/util/internal/protostream_objectwriter_test.cc index a9b15e68..97ef8fff 100644 --- a/src/google/protobuf/util/internal/protostream_objectwriter_test.cc +++ b/src/google/protobuf/util/internal/protostream_objectwriter_test.cc @@ -74,6 +74,8 @@ using google::protobuf::testing::Primitive; using google::protobuf::testing::Proto3Message; using google::protobuf::testing::Publisher; using google::protobuf::testing::StructType; +using google::protobuf::testing::TestJsonName1; +using google::protobuf::testing::TestJsonName2; using google::protobuf::testing::TimestampDuration; using google::protobuf::testing::ValueWrapper; using google::protobuf::testing::oneofs::OneOfsRequest; @@ -271,6 +273,26 @@ TEST_P(ProtoStreamObjectWriterTest, CustomJsonName) { CheckOutput(book); } +// Test that two messages can have different fields mapped to the same JSON +// name. See: https://github.com/google/protobuf/issues/1415 +TEST_P(ProtoStreamObjectWriterTest, ConflictingJsonName) { + ResetTypeInfo(TestJsonName1::descriptor()); + TestJsonName1 message1; + message1.set_one_value(12345); + ow_->StartObject("") + ->RenderInt32("value", 12345) + ->EndObject(); + CheckOutput(message1); + + ResetTypeInfo(TestJsonName2::descriptor()); + TestJsonName2 message2; + message2.set_another_value(12345); + ow_->StartObject("") + ->RenderInt32("value", 12345) + ->EndObject(); + CheckOutput(message2); +} + TEST_P(ProtoStreamObjectWriterTest, IntEnumValuesAreAccepted) { Book book; book.set_title("Some Book"); diff --git a/src/google/protobuf/util/internal/testdata/books.proto b/src/google/protobuf/util/internal/testdata/books.proto index 9fe4f7aa..869271f4 100644 --- a/src/google/protobuf/util/internal/testdata/books.proto +++ b/src/google/protobuf/util/internal/testdata/books.proto @@ -190,3 +190,12 @@ message Cyclic { repeated Author m_author = 5; optional Cyclic m_cyclic = 4; } + +// Test that two messages can have different fields mapped to the same JSON +// name. See: https://github.com/google/protobuf/issues/1415 +message TestJsonName1 { + optional int32 one_value = 1 [json_name = "value"]; +} +message TestJsonName2 { + optional int32 another_value = 1 [json_name = "value"]; +} diff --git a/src/google/protobuf/util/internal/type_info.cc b/src/google/protobuf/util/internal/type_info.cc index 17d58475..a5d903f1 100644 --- a/src/google/protobuf/util/internal/type_info.cc +++ b/src/google/protobuf/util/internal/type_info.cc @@ -107,12 +107,12 @@ class TypeInfoForTypeResolver : public TypeInfo { virtual const google::protobuf::Field* FindField( const google::protobuf::Type* type, StringPiece camel_case_name) const { - if (indexed_types_.find(type) == indexed_types_.end()) { - PopulateNameLookupTable(type); - indexed_types_.insert(type); - } + std::map<const google::protobuf::Type*, + CamelCaseNameTable>::const_iterator it = indexed_types_.find(type); + const CamelCaseNameTable& camel_case_name_table = (it == indexed_types_.end()) + ? PopulateNameLookupTable(type, &indexed_types_[type]) : it->second; StringPiece name = - FindWithDefault(camel_case_name_table_, camel_case_name, StringPiece()); + FindWithDefault(camel_case_name_table, camel_case_name, StringPiece()); if (name.empty()) { // Didn't find a mapping. Use whatever provided. name = camel_case_name; @@ -123,6 +123,7 @@ class TypeInfoForTypeResolver : public TypeInfo { private: typedef util::StatusOr<const google::protobuf::Type*> StatusOrType; typedef util::StatusOr<const google::protobuf::Enum*> StatusOrEnum; + typedef std::map<StringPiece, StringPiece> CamelCaseNameTable; template <typename T> static void DeleteCachedTypes(std::map<StringPiece, T>* cached_types) { @@ -134,32 +135,35 @@ class TypeInfoForTypeResolver : public TypeInfo { } } - void PopulateNameLookupTable(const google::protobuf::Type* type) const { + const CamelCaseNameTable& PopulateNameLookupTable( + const google::protobuf::Type* type, + CamelCaseNameTable* camel_case_name_table) const { for (int i = 0; i < type->fields_size(); ++i) { const google::protobuf::Field& field = type->fields(i); StringPiece name = field.name(); StringPiece camel_case_name = field.json_name(); const StringPiece* existing = InsertOrReturnExisting( - &camel_case_name_table_, camel_case_name, name); + camel_case_name_table, camel_case_name, name); if (existing && *existing != name) { GOOGLE_LOG(WARNING) << "Field '" << name << "' and '" << *existing << "' map to the same camel case name '" << camel_case_name << "'."; } } + return *camel_case_name_table; } TypeResolver* type_resolver_; // Stores string values that will be referenced by StringPieces in - // cached_types_, cached_enums_ and camel_case_name_table_. + // cached_types_, cached_enums_. mutable std::set<string> string_storage_; mutable std::map<StringPiece, StatusOrType> cached_types_; mutable std::map<StringPiece, StatusOrEnum> cached_enums_; - mutable std::set<const google::protobuf::Type*> indexed_types_; - mutable std::map<StringPiece, StringPiece> camel_case_name_table_; + mutable std::map<const google::protobuf::Type*, + CamelCaseNameTable> indexed_types_; }; } // namespace |