fix!: Raise AttributeError if an illegal attribute name is used

Signed-off-by: Daniel Azuma <[email protected]>

Author: dazuma

lib/cloud_events/event.rb

@@ -58,7 +58,7 @@ class << self
       # @param spec_version [String] The required `specversion` field.
       # @param kwargs [keywords] Additional parameters for the event.
       #
-      def create spec_version:, **kwargs
+      def create(spec_version:, **kwargs)
         case spec_version
         when "0.3"
           V0.new(spec_version: spec_version, **kwargs)

lib/cloud_events/event/field_interpreter.rb

@@ -14,8 +14,9 @@ def initialize(args)
         @attributes = {}
       end
 
-      def finish_attributes
+      def finish_attributes(requires_lc_start: false)
         @args.each do |key, value|
+          check_attribute_name(key, requires_lc_start)
           @attributes[key.freeze] = value.to_s.freeze unless value.nil?
         end
         @args = {}
@@ -131,6 +132,13 @@ def object(keys, required: false, allow_nil: false)
         @attributes[keys.first.freeze] = raw
         converted
       end
+
+      def check_attribute_name(key, requires_lc_start)
+        regex = requires_lc_start ? /^[a-z][a-z0-9]*$/ : /^[a-z0-9]+$/
+        unless regex.match?(key)
+          raise(AttributeError, "Illegal key: #{key.inspect} must consist only of digits and lower-case letters")
+        end
+      end
     end
   end
 end

lib/cloud_events/event/v0.rb

@@ -75,7 +75,7 @@ class V0
       #     (Also available using the deprecated keyword `attributes`.)
       # @param args [keywords] The data and attributes, as keyword arguments.
       #
-      def initialize set_attributes: nil, attributes: nil, **args
+      def initialize(set_attributes: nil, attributes: nil, **args)
         interpreter = FieldInterpreter.new(set_attributes || attributes || args)
         @spec_version = interpreter.spec_version(["specversion", "spec_version"], accept: /^0\.3$/)
         @id = interpreter.string(["id"], required: true)
@@ -88,7 +88,7 @@ def initialize set_attributes: nil, attributes: nil, **args
         @schema_url = interpreter.uri(["schemaurl", "schema_url"])
         @subject = interpreter.string(["subject"])
         @time = interpreter.rfc3339_date_time(["time"])
-        @attributes = interpreter.finish_attributes
+        @attributes = interpreter.finish_attributes(requires_lc_start: true)
         freeze
       end
 
@@ -101,7 +101,7 @@ def initialize set_attributes: nil, attributes: nil, **args
       # @param changes [keywords] See {#initialize} for a list of arguments.
       # @return [FunctionFramework::CloudEvents::Event]
       #
-      def with **changes
+      def with(**changes)
         attributes = @attributes.merge(changes)
         V0.new(set_attributes: attributes)
       end

lib/cloud_events/event/v1.rb

@@ -136,7 +136,7 @@ class V1
       #     (Also available using the deprecated keyword `attributes`.)
       # @param args [keywords] The data and attributes, as keyword arguments.
       #
-      def initialize set_attributes: nil, attributes: nil, **args
+      def initialize(set_attributes: nil, attributes: nil, **args)
         interpreter = FieldInterpreter.new(set_attributes || attributes || args)
         @spec_version = interpreter.spec_version(["specversion", "spec_version"], accept: /^1(\.|$)/)
         @id = interpreter.string(["id"], required: true)
@@ -167,7 +167,7 @@ def initialize set_attributes: nil, attributes: nil, **args
       # @param changes [keywords] See {#initialize} for a list of arguments.
       # @return [FunctionFramework::CloudEvents::Event]
       #
-      def with **changes
+      def with(**changes)
         changes = Utils.keys_to_strings(changes)
         attributes = @attributes.dup
         if changes.key?("data") || changes.key?("data_encoded")

lib/cloud_events/format.rb

@@ -79,7 +79,7 @@ module Format
     # @return [Hash] if accepting the request and returning a result
     # @return [nil] if declining the request.
     #
-    def decode_event **_kwargs
+    def decode_event(**_kwargs)
       nil
     end
 
@@ -116,7 +116,7 @@ def decode_event **_kwargs
     # @return [Hash] if accepting the request and returning a result
     # @return [nil] if declining the request.
     #
-    def encode_event **_kwargs
+    def encode_event(**_kwargs)
       nil
     end
 
@@ -155,7 +155,7 @@ def encode_event **_kwargs
     # @return [Hash] if accepting the request and returning a result
     # @return [nil] if declining the request.
     #
-    def decode_data **_kwargs
+    def decode_data(**_kwargs)
       nil
     end
 
@@ -194,7 +194,7 @@ def decode_data **_kwargs
     # @return [Hash] if accepting the request and returning a result
     # @return [nil] if declining the request.
     #
-    def encode_data **_kwargs
+    def encode_data(**_kwargs)
       nil
     end
 
@@ -227,7 +227,7 @@ def initialize(formats = [], &result_checker)
       ##
       # Implements {Format#decode_event}
       #
-      def decode_event **kwargs
+      def decode_event(**kwargs)
         @formats.each do |elem|
           result = elem.decode_event(**kwargs)
           result = @result_checker.call(result) if @result_checker
@@ -239,7 +239,7 @@ def decode_event **kwargs
       ##
       # Implements {Format#encode_event}
       #
-      def encode_event **kwargs
+      def encode_event(**kwargs)
         @formats.each do |elem|
           result = elem.encode_event(**kwargs)
           result = @result_checker.call(result) if @result_checker
@@ -251,7 +251,7 @@ def encode_event **kwargs
       ##
       # Implements {Format#decode_data}
       #
-      def decode_data **kwargs
+      def decode_data(**kwargs)
         @formats.each do |elem|
           result = elem.decode_data(**kwargs)
           result = @result_checker.call(result) if @result_checker
@@ -263,7 +263,7 @@ def decode_data **kwargs
       ##
       # Implements {Format#encode_data}
       #
-      def encode_data **kwargs
+      def encode_data(**kwargs)
         @formats.each do |elem|
           result = elem.encode_data(**kwargs)
           result = @result_checker.call(result) if @result_checker

lib/cloud_events/http_binding.rb

@@ -166,7 +166,7 @@ def probable_event?(env)
     # @raise [CloudEvents::CloudEventsError] if an event could not be decoded
     #     from the request.
     #
-    def decode_event env, allow_opaque: false, **format_args
+    def decode_event(env, allow_opaque: false, **format_args)
       request_method = env["REQUEST_METHOD"]
       raise(NotCloudEventError, "Request method is #{request_method}") if ILLEGAL_METHODS.include?(request_method)
       content_type_string = env["CONTENT_TYPE"]
@@ -204,7 +204,7 @@ def decode_event env, allow_opaque: false, **format_args
     # @param format_args [keywords] Extra args to pass to the formatter.
     # @return [Array(headers,String)]
     #
-    def encode_event event, structured_format: false, **format_args
+    def encode_event(event, structured_format: false, **format_args)
       if event.is_a?(Event::Opaque)
         [{ "Content-Type" => event.content_type.to_s }, event.content]
       elsif !structured_format
@@ -243,7 +243,7 @@ def encode_event event, structured_format: false, **format_args
     # @raise [CloudEvents::CloudEventsError] if the request appears to be a
     #     CloudEvent but decoding failed.
     #
-    def decode_rack_env env, **format_args
+    def decode_rack_env(env, **format_args)
       content_type_string = env["CONTENT_TYPE"]
       content_type = ContentType.new(content_type_string) if content_type_string
       content = read_with_charset(env["rack.input"], content_type&.charset)
@@ -262,7 +262,7 @@ def decode_rack_env env, **format_args
     # @param format_args [keywords] Extra args to pass to the formatter.
     # @return [Array(headers,String)]
     #
-    def encode_structured_content event, format_name, **format_args
+    def encode_structured_content(event, format_name, **format_args)
       result = @event_encoders[format_name]&.encode_event(event: event,
                                                           data_encoder: @data_encoders,
                                                           **format_args)
@@ -280,7 +280,7 @@ def encode_structured_content event, format_name, **format_args
     # @param format_args [keywords] Extra args to pass to the formatter.
     # @return [Array(headers,String)]
     #
-    def encode_batched_content event_batch, format_name, **format_args
+    def encode_batched_content(event_batch, format_name, **format_args)
       result = @event_encoders[format_name]&.encode_event(event_batch: event_batch,
                                                           data_encoder: @data_encoders,
                                                           **format_args)
@@ -297,7 +297,7 @@ def encode_batched_content event_batch, format_name, **format_args
     # @param format_args [keywords] Extra args to pass to the formatter.
     # @return [Array(headers,String)]
     #
-    def encode_binary_content event, legacy_data_encode: true, **format_args
+    def encode_binary_content(event, legacy_data_encode: true, **format_args)
       headers = {}
       event.to_h.each do |key, value|
         unless ["data", "data_encoded", "datacontenttype"].include?(key)
@@ -369,7 +369,7 @@ def add_named_formatter(collection, formatter, name)
     # Decode a single event from the given request body and content type in
     # structured mode.
     #
-    def decode_structured_content content, content_type, allow_opaque, **format_args
+    def decode_structured_content(content, content_type, allow_opaque, **format_args)
       result = @event_decoders.decode_event(content: content,
                                             content_type: content_type,
                                             data_decoder: @data_decoders,
@@ -389,7 +389,7 @@ def decode_structured_content content, content_type, allow_opaque, **format_args
     # TODO: legacy_data_decode is deprecated and can be removed when
     # decode_rack_env is removed.
     #
-    def decode_binary_content content, content_type, env, legacy_data_decode, **format_args
+    def decode_binary_content(content, content_type, env, legacy_data_decode, **format_args)
       spec_version = env["HTTP_CE_SPECVERSION"]
       return nil unless spec_version
       unless spec_version =~ /^0\.3|1(\.|$)/
@@ -479,13 +479,13 @@ def read_with_charset(io, charset)
     # @private
     module DefaultDataFormat
       # @private
-      def self.decode_data content: nil, content_type: nil, **_extra_kwargs
+      def self.decode_data(content: nil, content_type: nil, **_extra_kwargs)
         return nil unless content_type.nil?
         { data: content, content_type: nil }
       end
 
       # @private
-      def self.encode_data data: nil, content_type: nil, **_extra_kwargs
+      def self.encode_data(data: nil, content_type: nil, **_extra_kwargs)
         return nil unless content_type.nil?
         { content: data.to_s, content_type: nil }
       end

lib/cloud_events/json_format.rb

@@ -37,7 +37,7 @@ class JsonFormat
     # @raise [CloudEvents::SpecVersionError] if an unsupported specversion is
     #     found.
     #
-    def decode_event content: nil, content_type: nil, data_decoder: nil, **_other_kwargs
+    def decode_event(content: nil, content_type: nil, data_decoder: nil, **_other_kwargs)
       return nil unless content && content_type&.media_type == "application" && content_type&.subtype_format == "json"
       case content_type.subtype_base
       when "cloudevents"
@@ -77,7 +77,7 @@ def decode_event content: nil, content_type: nil, data_decoder: nil, **_other_kw
     # @return [nil] if declining the request.
     # @raise [CloudEvents::FormatSyntaxError] if the JSON could not be parsed
     #
-    def encode_event event: nil, event_batch: nil, data_encoder: nil, sort: false, **_other_kwargs
+    def encode_event(event: nil, event_batch: nil, data_encoder: nil, sort: false, **_other_kwargs)
       if event && !event_batch
         structure = encode_hash_structure(event, data_encoder: data_encoder)
         structure = sort_keys(structure) if sort
@@ -119,7 +119,7 @@ def encode_event event: nil, event_batch: nil, data_encoder: nil, sort: false, *
     # @raise [CloudEvents::SpecVersionError] if an unsupported specversion is
     #     found.
     #
-    def decode_data spec_version: nil, content: nil, content_type: nil, **_other_kwargs
+    def decode_data(spec_version: nil, content: nil, content_type: nil, **_other_kwargs)
       return nil unless spec_version
       return nil unless content
       return nil unless json_content_type?(content_type)
@@ -154,7 +154,7 @@ def decode_data spec_version: nil, content: nil, content_type: nil, **_other_kwa
     # @return [Hash] if accepting the request.
     # @return [nil] if declining the request.
     #
-    def encode_data spec_version: nil, data: UNSPECIFIED, content_type: nil, sort: false, **_other_kwargs
+    def encode_data(spec_version: nil, data: UNSPECIFIED, content_type: nil, sort: false, **_other_kwargs)
       return nil unless spec_version
       return nil if data == UNSPECIFIED
       return nil unless json_content_type?(content_type)

lib/cloud_events/text_format.rb

@@ -30,7 +30,7 @@ class TextFormat
     # @return [Hash] if accepting the request.
     # @return [nil] if declining the request.
     #
-    def decode_data content: nil, content_type: nil, **_other_kwargs
+    def decode_data(content: nil, content_type: nil, **_other_kwargs)
       return nil unless content
       return nil unless text_content_type?(content_type)
       { data: content.to_s, content_type: content_type }
@@ -56,7 +56,7 @@ def decode_data content: nil, content_type: nil, **_other_kwargs
     # @return [Hash] if accepting the request.
     # @return [nil] if declining the request.
     #
-    def encode_data data: UNSPECIFIED, content_type: nil, **_other_kwargs
+    def encode_data(data: UNSPECIFIED, content_type: nil, **_other_kwargs)
       return nil if data == UNSPECIFIED
       return nil unless text_content_type?(content_type)
       { content: data.to_s, content_type: content_type }

test/event/test_v0.rb

@@ -211,6 +211,25 @@
     assert_equal "The type field is required", error.message
   end
 
+  it "validates attribute name" do
+    error = assert_raises(CloudEvents::AttributeError) do
+      CloudEvents::Event::V0.new(id:           my_id,
+                                 source:       my_source,
+                                 type:         my_type,
+                                 spec_version: spec_version,
+                                 "1parent":    my_trace_parent)
+    end
+    assert_includes error.message, "Illegal key: \"1parent\""
+    error = assert_raises(CloudEvents::AttributeError) do
+      CloudEvents::Event::V0.new(id:           my_id,
+                                 source:       my_source,
+                                 type:         my_type,
+                                 spec_version: spec_version,
+                                 trace_parent: my_trace_parent)
+    end
+    assert_includes error.message, "Illegal key: \"trace_parent\""
+  end
+
   it "handles extension attributes" do
     event = CloudEvents::Event::V0.new(id:           my_id,
                                        source:       my_source,

test/event/test_v1.rb

@@ -272,6 +272,22 @@
     assert_equal "The type field is required", error.message
   end
 
+  it "validates attribute name" do
+    CloudEvents::Event::V1.new(id:           my_id,
+                               source:       my_source,
+                               type:         my_type,
+                               spec_version: spec_version,
+                               "1parent":    my_trace_parent)
+    error = assert_raises(CloudEvents::AttributeError) do
+      CloudEvents::Event::V1.new(id:           my_id,
+                                 source:       my_source,
+                                 type:         my_type,
+                                 spec_version: spec_version,
+                                 trace_parent: my_trace_parent)
+    end
+    assert_includes error.message, "Illegal key: \"trace_parent\""
+  end
+
   it "handles extension attributes" do
     event = CloudEvents::Event::V1.new(id:           my_id,
                                        source:       my_source,