Ensure DBus.type constructs a valid Type.

Previously it was easy to construct an invalid signature,
even for trivially bad cases.

Now, make sure `DBus.type` constructs a valid Single Complete Type,
and add `DBus.types` for constructing a valid Signature (0 or more SCTs).

Add test cases.

The corresponding section of the specification is
https://dbus.freedesktop.org/doc/dbus-specification.html#type-system

Also, `DBus::Type` was an internal Module and the constructed Class
was awkwardly named `DBus::Type::Type`.
Merged them (with a compatibility "symlink")

Author: mvidner

NEWS.md

@@ -2,7 +2,12 @@
 
 ## Unreleased
 
+API:
+ * Renamed the DBus::Type::Type class to DBus::Type
+   (which was previously a module).
+
 Bug fixes:
+ * Signature validation: Ensure DBus.type produces a valid Type
  * Detect more malformed messages: non-NUL padding bytes, variants with
    multiple or no value.
 

doc/Reference.md

@@ -171,7 +171,7 @@ by the D-Bus signature.
 If the signature expects a Variant
 (which is the case for all Properties!) then an explicit mechanism is needed.
 
-1. A pair [{DBus::Type::Type}, value] specifies to marshall *value* as
+1. A pair [{DBus::Type}, value] specifies to marshall *value* as
    that specified type.
    The pair can be produced by {DBus.variant}(signature, value) which
    gives the  same result as [{DBus.type}(signature), value].

lib/dbus/marshall.rb

@@ -403,7 +403,7 @@ def append_variant(val)
       vartype = nil
       if val.is_a?(Array) && val.size == 2
         case val[0]
-        when DBus::Type::Type
+        when Type
           vartype, vardata = val
         when String
           begin
@@ -427,7 +427,7 @@ def append_variant(val)
       @packet += sub.packet
     end
 
-    # @param child_type [DBus::Type::Type]
+    # @param child_type [Type]
     def append_array(child_type, val)
       if val.is_a?(Hash)
         raise TypeException, "Expected an Array but got a Hash" if child_type.sigtype != Type::DICT_ENTRY

lib/dbus/type.rb

@@ -33,7 +33,8 @@ class Prototype < String; end
   #
   # This module containts the constants of the types specified in the D-Bus
   # protocol.
-  module Type
+  #
+  class Type
     # Mapping from type number to name and alignment.
     TYPE_MAPPING = {
       0 => ["INVALID", nil],
@@ -64,91 +65,117 @@ module Type
     class SignatureException < Exception
     end
 
-    # = D-Bus type conversion class
-    #
-    # Helper class for representing a D-Bus type.
-    class Type
-      # Returns the signature type number.
-      attr_reader :sigtype
-      # Return contained member types.
-      attr_reader :members
-
-      # Create a new type instance for type number _sigtype_.
-      def initialize(sigtype)
-        if !TYPE_MAPPING.keys.member?(sigtype)
-          raise SignatureException, "Unknown key in signature: #{sigtype.chr}"
-        end
+    # Formerly this was a Module and there was a DBus::Type::Type class
+    # but the class got too prominent to keep its double double name.
+    # This is for backward compatibility.
+    Type = self # rubocop:disable Naming/ConstantName
 
-        @sigtype = sigtype
-        @members = []
-      end
+    # Returns the signature type number.
+    attr_reader :sigtype
+    # Return contained member types.
+    attr_reader :members
 
-      # Return the required alignment for the type.
-      def alignment
-        TYPE_MAPPING[@sigtype].last
+    # Use {DBus.type} instead, because this allows constructing
+    # incomplete or invalid types, for backward compatibility.
+    #
+    # @param abstract [Boolean] allow abstract types "r" and "e"
+    #   (Enabled for internal usage by {Parser}.)
+    def initialize(sigtype, abstract: false)
+      if !TYPE_MAPPING.keys.member?(sigtype)
+        case sigtype
+        when ")"
+          raise SignatureException, "STRUCT unexpectedly closed: )"
+        when "}"
+          raise SignatureException, "DICT_ENTRY unexpectedly closed: }"
+        else
+          raise SignatureException, "Unknown type code #{sigtype.inspect}"
+        end
       end
 
-      # Return a string representation of the type according to the
-      # D-Bus specification.
-      def to_s
-        case @sigtype
+      unless abstract
+        case sigtype
         when STRUCT
-          "(#{@members.collect(&:to_s).join})"
-        when ARRAY
-          "a#{child}"
+          raise SignatureException, "Abstract STRUCT, use \"(...)\" instead of \"#{STRUCT}\""
         when DICT_ENTRY
-          "{#{@members.collect(&:to_s).join}}"
-        else
-          if !TYPE_MAPPING.keys.member?(@sigtype)
-            raise NotImplementedError
-          end
-
-          @sigtype.chr
+          raise SignatureException, "Abstract DICT_ENTRY, use \"{..}\" instead of \"#{DICT_ENTRY}\""
         end
       end
 
-      # Add a new member type _item_.
-      def <<(item)
-        if ![STRUCT, ARRAY, DICT_ENTRY].member?(@sigtype)
-          raise SignatureException
-        end
-        raise SignatureException if @sigtype == ARRAY && !@members.empty?
-
-        if @sigtype == DICT_ENTRY
-          case @members.size
-          when 2
-            raise SignatureException, "Dict entries have exactly two members"
-          when 0
-            if [STRUCT, ARRAY, DICT_ENTRY].member?(item.sigtype)
-              raise SignatureException, "Dict entry keys must be basic types"
-            end
-          end
+      @sigtype = sigtype
+      @members = []
+    end
+
+    # Return the required alignment for the type.
+    def alignment
+      TYPE_MAPPING[@sigtype].last
+    end
+
+    # Return a string representation of the type according to the
+    # D-Bus specification.
+    def to_s
+      case @sigtype
+      when STRUCT
+        "(#{@members.collect(&:to_s).join})"
+      when ARRAY
+        "a#{child}"
+      when DICT_ENTRY
+        "{#{@members.collect(&:to_s).join}}"
+      else
+        if !TYPE_MAPPING.keys.member?(@sigtype)
+          raise NotImplementedError
         end
-        @members << item
+
+        @sigtype.chr
       end
+    end
 
-      # Return the first contained member type.
-      def child
-        @members[0]
+    # Add a new member type _item_.
+    def <<(item)
+      if ![STRUCT, ARRAY, DICT_ENTRY].member?(@sigtype)
+        raise SignatureException
       end
+      raise SignatureException if @sigtype == ARRAY && !@members.empty?
 
-      def inspect
-        s = TYPE_MAPPING[@sigtype].first
-        if [STRUCT, ARRAY].member?(@sigtype)
-          s += ": #{@members.inspect}"
+      if @sigtype == DICT_ENTRY
+        case @members.size
+        when 2
+          raise SignatureException, "DICT_ENTRY must have 2 subtypes, found 3 or more in #{@signature}"
+        when 0
+          if [STRUCT, ARRAY, DICT_ENTRY, VARIANT].member?(item.sigtype)
+            raise SignatureException, "DICT_ENTRY key must be basic (non-container)"
+          end
         end
-        s
       end
+      @members << item
+    end
+
+    # Return the first contained member type.
+    def child
+      @members[0]
+    end
+
+    def inspect
+      s = TYPE_MAPPING[@sigtype].first
+      if [STRUCT, ARRAY].member?(@sigtype)
+        s += ": #{@members.inspect}"
+      end
+      s
     end
 
     # = D-Bus type parser class
     #
     # Helper class to parse a type signature in the protocol.
+    # @api private
     class Parser
       # Create a new parser for the given _signature_.
       # @param signature [Signature]
       def initialize(signature)
         @signature = signature
+        if signature.size > 255
+          msg = "Potential signature is longer than 255 characters (#{@signature.size}): #{@signature}"
+          raise SignatureException, msg
+        end
+
         @idx = 0
       end
 
@@ -160,35 +187,52 @@ def nextchar
       end
 
       # Parse one character _char_ of the signature.
-      def parse_one(char)
+      # @param for_array [Boolean] are we parsing an immediate child of an ARRAY
+      def parse_one(char, for_array: false)
         res = nil
         case char
         when "a"
           res = Type.new(ARRAY)
           char = nextchar
-          raise SignatureException, "Parse error in #{@signature}" if char.nil?
+          raise SignatureException, "Empty ARRAY in #{@signature}" if char.nil?
 
-          child = parse_one(char)
+          child = parse_one(char, for_array: true)
           res << child
         when "("
-          res = Type.new(STRUCT)
+          res = Type.new(STRUCT, abstract: true)
           while (char = nextchar) && char != ")"
             res << parse_one(char)
           end
-          raise SignatureException, "Parse error in #{@signature}" if char.nil?
+          raise SignatureException, "STRUCT not closed in #{@signature}" if char.nil?
+          raise SignatureException, "Empty STRUCT in #{@signature}" if res.members.empty?
         when "{"
-          res = Type.new(DICT_ENTRY)
-          while (char = nextchar) && char != "}"
+          raise SignatureException, "DICT_ENTRY not an immediate child of an ARRAY" unless for_array
+
+          res = Type.new(DICT_ENTRY, abstract: true)
+
+          # key type, value type
+          2.times do |i|
+            char = nextchar
+            raise SignatureException, "DICT_ENTRY not closed in #{@signature}" if char.nil?
+
+            raise SignatureException, "DICT_ENTRY must have 2 subtypes, found #{i} in #{@signature}" if char == "}"
+
             res << parse_one(char)
           end
-          raise SignatureException, "Parse error in #{@signature}" if char.nil?
+
+          # closing "}"
+          char = nextchar
+          raise SignatureException, "DICT_ENTRY not closed in #{@signature}" if char.nil?
+
+          raise SignatureException, "DICT_ENTRY must have 2 subtypes, found 3 or more in #{@signature}" if char != "}"
         else
           res = Type.new(char)
         end
         res
       end
 
       # Parse the entire signature, return a DBus::Type object.
+      # @return [Array<Type>]
       def parse
         @idx = 0
         ret = []
@@ -197,17 +241,43 @@ def parse
         end
         ret
       end
+
+      # Parse one {SingleCompleteType}
+      # @return [Type]
+      def parse1
+        c = nextchar
+        raise SignatureException, "Empty signature, expecting a Single Complete Type" if c.nil?
+
+        t = parse_one(c)
+        raise SignatureException, "Has more than a Single Complete Type: #{@signature}" unless nextchar.nil?
+
+        t
+      end
     end
   end
 
   # shortcuts
 
-  # Parse a String to a DBus::Type::Type
+  # Parse a String to a valid {DBus::Type}.
+  # This is prefered to {Type#initialize} which allows
+  # incomplete or invalid types.
+  # @param string_type [SingleCompleteType]
+  # @return [DBus::Type]
+  # @raise SignatureException
   def type(string_type)
-    Type::Parser.new(string_type).parse[0]
+    Type::Parser.new(string_type).parse1
   end
   module_function :type
 
+  # Parse a String to zero or more {DBus::Type}s.
+  # @param string_type [Signature]
+  # @return [Array<DBus::Type>]
+  # @raise SignatureException
+  def types(string_type)
+    Type::Parser.new(string_type).parse
+  end
+  module_function :types
+
   # Make an explicit [Type, value] pair
   def variant(string_type, value)
     [type(string_type), value]