Fix bug in encode_uint64, value zero should encode zero bytes

Shopify · Apr 3, 2024 · d57e7a2 · d57e7a2
1 parent f871549
commit d57e7a2
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 17 deletions.
diff --git a/lib/protoboeuf/codegen.rb b/lib/protoboeuf/codegen.rb
@@ -115,9 +115,9 @@ def encode_bool(field)
         tag = (field.number << 3) | field.wire_type
         # False/zero is the default value, so the false case encodes nothing
         <<-eocode
-        ## encode the tag
         val = @#{field.name}
         if val == true
+          ## encode the tag
           buff << #{sprintf("%#04x", tag)}
           buff << 1
         elsif val == false
@@ -130,15 +130,18 @@ def encode_bool(field)
 
       def encode_uint64(field)
         tag = (field.number << 3) | field.wire_type
+        # Zero is the default value, so it encodes zero bytes
         <<-eocode
-        ## encode the tag
-        buff << #{sprintf("%#04x", tag)}
         val = @#{field.name}
-        while val > 0
-          byte = val & 0x7F
-          val >>= 7
-          byte |= 0x80 if val > 0
-          buff << byte
+        if val != 0
+          ## encode the tag
+          buff << #{sprintf("%#04x", tag)}
+          while val > 0
+            byte = val & 0x7F
+            val >>= 7
+            byte |= 0x80 if val > 0
+            buff << byte
+          end
         end
         eocode
       end

diff --git a/test/message_test.rb b/test/message_test.rb
@@ -606,14 +606,10 @@ def test_encode_uint64
 
     m = Module.new { class_eval code.to_ruby }
 
-    # Should fit in one byte
-    actual = m::UInt64Value.encode m::UInt64Value.new(value: 12)
-    expected = ::Google::Protobuf::UInt64Value.encode(::Google::Protobuf::UInt64Value.new(value: 12))
-    assert_equal expected, actual
-
-    # Use more bytes
-    actual = m::UInt64Value.encode m::UInt64Value.new(value: 0xFF)
-    expected = ::Google::Protobuf::UInt64Value.encode(::Google::Protobuf::UInt64Value.new(value: 0xFF))
-    assert_equal expected, actual
+    [0, 12, 0xFF, 0xFFFF_FFFF_FFFF_FFFF].each do |n|
+      actual = m::UInt64Value.encode m::UInt64Value.new(value: n)
+      expected = ::Google::Protobuf::UInt64Value.encode(::Google::Protobuf::UInt64Value.new(value: n))
+      assert_equal expected, actual
+    end
   end
 end