Fix: Repeated float values weren't being treated as arrays and were c…

…rashing.
flipp-oss · Sep 10, 2024 · f61333a · f61333a
1 parent ff71d38
commit f61333a
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 42 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## UNRELEASED
 
+# 0.1.20 - 2024-09-10
+- Fix: Repeated float values weren't being treated as arrays and were crashing.
+
 # 0.1.19 - 2024-08-22
 
 - Add `emit_defaults` extension for JSON responses.

diff --git a/lib/grpc_rest.rb b/lib/grpc_rest.rb
@@ -63,56 +63,55 @@ def handle_enum_values(descriptor, value)
       end
     end
 
-    def map_proto_type(proto, params)
+    def map_proto_type(descriptor, val)
+      if descriptor.subtype.is_a?(Google::Protobuf::EnumDescriptor)
+        return handle_enum_values(descriptor, val)
+      end
+
+      case descriptor.type
+      when *%i(int32 int64 uint32 uint64 sint32 sint64 fixed32 fixed64 sfixed32 sfixed64)
+        return val.to_i
+      when *%i(float double)
+        return val.to_f
+      when :bool
+        return !!val
+      end
+
+      case descriptor.subtype&.name
+      when 'google.protobuf.Struct'
+        return Google::Protobuf::Struct.from_hash(val)
+      when 'google.protobuf.Timestamp'
+        if val.is_a?(Numeric)
+          return Google::Protobuf::Timestamp.from_time(Time.at(val))
+        else
+          return Google::Protobuf::Timestamp.from_time(Time.new(val))
+        end
+      when 'google.protobuf.Value'
+        return Google::Protobuf::Value.from_ruby(val)
+      when 'google.protobuf.ListValue'
+        return Google::Protobuf::ListValue.from_a(val)
+      else
+        return map_proto_record(descriptor.subtype, val)
+      end
+    end
+
+    def map_proto_record(proto, params)
       proto.to_a.each do |descriptor|
         field = descriptor.name
         val = params[field]
         next if val.nil?
-        if descriptor.subtype.is_a?(Google::Protobuf::EnumDescriptor)
-          if descriptor.label == :repeated
-            params[field] = val.map { |v| handle_enum_values(descriptor, v)}
-          else
-            params[field] = handle_enum_values(descriptor, val)
-          end
-          next
-        end
-
-        case descriptor.type
-        when *%i(int32 int64 uint32 uint64 sint32 sint64 fixed32 fixed64 sfixed32 sfixed64)
-          params[field] = val.to_i
-        when *%i(float double)
-          params[field] = val.to_f
-        when :bool
-          params[field] = !!val
-        end
 
-        case descriptor.subtype&.name
-        when 'google.protobuf.Struct'
-          params[field] = Google::Protobuf::Struct.from_hash(val)
-        when 'google.protobuf.Timestamp'
-          if val.is_a?(Numeric)
-            params[field] = Google::Protobuf::Timestamp.from_time(Time.at(val))
-          else
-            params[field] = Google::Protobuf::Timestamp.from_time(Time.new(val))
-          end
-        when 'google.protobuf.Value'
-          params[field] = Google::Protobuf::Value.from_ruby(val)
-        when 'google.protobuf.ListValue'
-          params[field] = Google::Protobuf::ListValue.from_a(val)
+        if descriptor.label == :repeated
+          params[field] = val.map { |v| map_proto_type(descriptor, v) }
         else
-          if params[field].is_a?(Array)
-            params[field].each do |item|
-              map_proto_type(descriptor.subtype, item)
-            end
-          else
-            map_proto_type(descriptor.subtype, params[field])
-          end
+          params[field] = map_proto_type(descriptor, val)
         end
       end
+      params
     end
 
     def init_request(request_class, params)
-      map_proto_type(request_class.descriptor, params)
+      map_proto_record(request_class.descriptor, params)
       request_class.decode_json(JSON.generate(params), ignore_unknown_fields: true)
     end
 

diff --git a/protoc-gen-rails/testdata/test_service.proto b/protoc-gen-rails/testdata/test_service.proto
@@ -20,6 +20,7 @@ message TestRequest {
   repeated SubRecord sub_records = 10;
   int32 some_int = 11;
   TestEnum some_enum = 12;
+  repeated float repeated_float = 13;
 }
 
 message SubRecord {

diff --git a/spec/grpc_rest_spec.rb b/spec/grpc_rest_spec.rb
@@ -67,6 +67,7 @@ def test_4(req)
     let(:params) do
       {
         test_id: 'abc',
+        repeated_float: [1.0, 2.0],
         some_int: "65",
         foobar: 'xyz',
         repeated_string: ['W', 'T', 'F'],
@@ -101,7 +102,7 @@ def test_4(req)
       expect(response).to be_successful
       expect(response.parsed_body).to eq({
                                            'someInt' => 4,
-                                           'fullResponse' => %({"testId":"abc","foobar":"xyz","repeatedString":["W","T","F"],"subRecord":{"subId":"id1","anotherId":"id2"},"secondRecord":{"subId":"id3","anotherId":"id4"},"structField":{"bool_key":true,"str_key":"val","nil_key":null,"list_key":[{"inner_key":"inner_val"}],"int_key":123},"timestampField":"2024-04-03T01:02:03Z","listValue":["F","Y","I"],"bareValue":45,"someInt":65,"someEnum":"TEST_ENUM_FOO"})
+                                           'fullResponse' => %({"testId":"abc","foobar":"xyz","repeatedString":["W","T","F"],"subRecord":{"subId":"id1","anotherId":"id2"},"secondRecord":{"subId":"id3","anotherId":"id4"},"structField":{"bool_key":true,"str_key":"val","nil_key":null,"list_key":[{"inner_key":"inner_val"}],"int_key":123},"timestampField":"2024-04-03T01:02:03Z","listValue":["F","Y","I"],"bareValue":45,"someInt":65,"someEnum":"TEST_ENUM_FOO","repeatedFloat":[1,2]})
                                          })
     end
 
@@ -111,7 +112,7 @@ def test_4(req)
       expect(response).to be_successful
       expect(response.parsed_body).to eq({
                                            'someInt' => 4,
-                                           'fullResponse' => %({"testId":"abc","foobar":"xyz","repeatedString":["W","T","F"],"subRecord":{"subId":"id1","anotherId":"id2"},"secondRecord":{"subId":"id3","anotherId":"id4"},"structField":{"bool_key":true,"str_key":"val","nil_key":null,"list_key":[{"inner_key":"inner_val"}],"int_key":123},"timestampField":"2024-04-03T01:02:03Z","listValue":["F","Y","I"],"bareValue":45,"someInt":65,"someEnum":"TEST_ENUM_FOO"})
+                                           'fullResponse' => %({"testId":"abc","foobar":"xyz","repeatedString":["W","T","F"],"subRecord":{"subId":"id1","anotherId":"id2"},"secondRecord":{"subId":"id3","anotherId":"id4"},"structField":{"bool_key":true,"str_key":"val","nil_key":null,"list_key":[{"inner_key":"inner_val"}],"int_key":123},"timestampField":"2024-04-03T01:02:03Z","listValue":["F","Y","I"],"bareValue":45,"someInt":65,"someEnum":"TEST_ENUM_FOO","repeatedFloat":[1,2]})
                                          })
     end
   end

diff --git a/spec/test_service_pb.rb b/spec/test_service_pb.rb