Skip to content

Commit

Permalink
Fix bug with large extension field numbers.
Browse files Browse the repository at this point in the history
Previously, extensions with field numbers greater than 268435455 would
result in a compile time error in generated code that looks something
like this:

Foo.java:3178: error: integer number too large: 3346754610
                3346754610);

This is because we were trying to represent the tag number (an
unsigned int) using a java int constant, but java int constants are
signed, and can't exceed Integer.MAX_VALUE.

Fixed by declaring it as a long instead, and casting it down to an
int in the implementation. This is safe, because the tag value always
fits in 32 bis.

Change-Id: If2017bacb4e20af667eaeaf9b65ddc2c30a7709f
  • Loading branch information
brianduff committed Apr 28, 2015
1 parent fe7b566 commit ec2f244
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 9 deletions.
22 changes: 14 additions & 8 deletions javanano/src/main/java/com/google/protobuf/nano/Extension.java
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,20 @@ public class Extension<M extends ExtendableMessageNano<M>, T> {
public static final int TYPE_SINT32 = InternalNano.TYPE_SINT32;
public static final int TYPE_SINT64 = InternalNano.TYPE_SINT64;

// Note: these create...() methods take a long for the tag parameter,
// because tags are represented as unsigned longs, and these values exist
// in generated code as long values. However, they can fit in 32-bits, so
// it's safe to cast them to int without loss of precision.

/**
* Creates an {@code Extension} of the given message type and tag number.
* Should be used by the generated code only.
*
* @param type {@link #TYPE_MESSAGE} or {@link #TYPE_GROUP}
*/
public static <M extends ExtendableMessageNano<M>, T extends MessageNano>
Extension<M, T> createMessageTyped(int type, Class<T> clazz, int tag) {
return new Extension<M, T>(type, clazz, tag, false);
Extension<M, T> createMessageTyped(int type, Class<T> clazz, long tag) {
return new Extension<M, T>(type, clazz, (int) tag, false);
}

/**
Expand All @@ -92,8 +97,8 @@ Extension<M, T> createMessageTyped(int type, Class<T> clazz, int tag) {
* @param type {@link #TYPE_MESSAGE} or {@link #TYPE_GROUP}
*/
public static <M extends ExtendableMessageNano<M>, T extends MessageNano>
Extension<M, T[]> createRepeatedMessageTyped(int type, Class<T[]> clazz, int tag) {
return new Extension<M, T[]>(type, clazz, tag, true);
Extension<M, T[]> createRepeatedMessageTyped(int type, Class<T[]> clazz, long tag) {
return new Extension<M, T[]>(type, clazz, (int) tag, true);
}

/**
Expand All @@ -104,8 +109,8 @@ Extension<M, T[]> createRepeatedMessageTyped(int type, Class<T[]> clazz, int tag
* @param clazz the boxed Java type of this extension
*/
public static <M extends ExtendableMessageNano<M>, T>
Extension<M, T> createPrimitiveTyped(int type, Class<T> clazz, int tag) {
return new PrimitiveExtension<M, T>(type, clazz, tag, false, 0, 0);
Extension<M, T> createPrimitiveTyped(int type, Class<T> clazz, long tag) {
return new PrimitiveExtension<M, T>(type, clazz, (int) tag, false, 0, 0);
}

/**
Expand All @@ -117,8 +122,9 @@ Extension<M, T> createPrimitiveTyped(int type, Class<T> clazz, int tag) {
*/
public static <M extends ExtendableMessageNano<M>, T>
Extension<M, T> createRepeatedPrimitiveTyped(
int type, Class<T> clazz, int tag, int nonPackedTag, int packedTag) {
return new PrimitiveExtension<M, T>(type, clazz, tag, true, nonPackedTag, packedTag);
int type, Class<T> clazz, long tag, long nonPackedTag, long packedTag) {
return new PrimitiveExtension<M, T>(type, clazz, (int) tag, true,
(int) nonPackedTag, (int) packedTag);
}

/**
Expand Down
13 changes: 13 additions & 0 deletions javanano/src/main/java/com/google/protobuf/nano/Extension.java.rej
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
diff a/javanano/src/main/java/com/google/protobuf/nano/Extension.java b/javanano/src/main/java/com/google/protobuf/nano/Extension.java (rejected hunks)
@@ -74,6 +74,11 @@ public class Extension<M extends ExtendableMessageNano<M>, T> {
public static final int TYPE_SINT32 = 17;
public static final int TYPE_SINT64 = 18;

+ // Note: these create...() methods take a long for the tag parameter,
+ // because tags are represented as unsigned longs, and these values exist
+ // in generated code as long values. However, they can fit in 32-bits, so
+ // it's safe to cast them to int without loss of precision.
+
/**
* Creates an {@code Extension} of the given message type and tag number.
* Should be used by the generated code only.
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ message AnotherMessage {
message ContainerMessage {
extend ExtendableMessage {
optional bool another_thing = 100;
// The largest permitted field number, per
// https://developers.google.com/protocol-buffers/docs/proto#simple
optional bool large_field_number = 536870911;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ void ExtensionGenerator::Generate(io::Printer* printer) const {
" com.google.protobuf.nano.Extension.create$repeated$$ext_type$(\n"
" com.google.protobuf.nano.Extension.$type$,\n"
" $class$.class,\n"
" $tag_params$);\n");
" $tag_params$L);\n");
}

} // namespace javanano
Expand Down

0 comments on commit ec2f244

Please sign in to comment.