🔀 Merge pull request #198 from ruby/validate-response_tagged-in-the-parser

🥅 Validate response tagged in the parser

Author: nevans

lib/net/imap.rb

@@ -2519,7 +2519,8 @@ def get_tagged_response(tag, cmd, timeout = nil)
       when /\A(?:BAD)\z/ni
         raise BadResponseError, resp
       else
-        raise UnknownResponseError, resp
+        disconnect
+        raise InvalidResponseError, "invalid tagged resp: %p" % [resp.raw.chomp]
       end
     end
 

lib/net/imap/errors.rb

@@ -47,7 +47,27 @@ class BadResponseError < ResponseError
     class ByeResponseError < ResponseError
     end
 
+    # Error raised when the server sends an invalid response.
+    #
+    # This is different from UnknownResponseError: the response has been
+    # rejected.  Although it may be parsable, the server is forbidden from
+    # sending it in the current context.  The client should automatically
+    # disconnect, abruptly (without logout).
+    #
+    # Note that InvalidResponseError does not inherit from ResponseError: it
+    # can be raised before the response is fully parsed.  A related
+    # ResponseParseError or ResponseError may be the #cause.
+    class InvalidResponseError < Error
+    end
+
     # Error raised upon an unknown response from the server.
+    #
+    # This is different from InvalidResponseError: the response may be a
+    # valid extension response and the server may be allowed to send it in
+    # this context, but Net::IMAP either does not know how to parse it or
+    # how to handle it.  This could result from enabling unknown or
+    # unhandled extensions.  The connection may still be usable,
+    # but—depending on context—it may be prudent to disconnect.
     class UnknownResponseError < ResponseError
     end
 

lib/net/imap/response_parser.rb

@@ -58,6 +58,21 @@ def parse(str)
       T_TEXT     = :TEXT         # any char except CRLF
       T_EOF      = :EOF          # end of response string
 
+      module ResponseConditions
+        OK      = "OK"
+        NO      = "NO"
+        BAD     = "BAD"
+        BYE     = "BYE"
+        PREAUTH = "PREAUTH"
+
+        RESP_COND_STATES      = [OK, NO, BAD              ].freeze
+        RESP_DATA_CONDS       = [OK, NO, BAD, BYE,        ].freeze
+        AUTH_CONDS            = [OK,               PREAUTH].freeze
+        GREETING_CONDS        = [OK,          BYE, PREAUTH].freeze
+        RESP_CONDS            = [OK, NO, BAD, BYE, PREAUTH].freeze
+      end
+      include ResponseConditions
+
       module Patterns
 
         module CharClassSubtraction
@@ -320,10 +335,13 @@ def unescape_quoted(quoted)
 
       ASTRING_TOKENS = [T_QUOTED, *ASTRING_CHARS_TOKENS, T_LITERAL].freeze
 
-      # atom            = 1*ATOM-CHAR
-      #
-      # TODO: match atom entirely by regexp (in the "lexer")
-      def atom; -combine_adjacent(*ATOM_TOKENS) end
+      # tag             = 1*<any ASTRING-CHAR except "+">
+      TAG_TOKENS = (ASTRING_CHARS_TOKENS - [T_PLUS]).freeze
+
+      # TODO: handle atom, astring_chars, and tag entirely inside the lexer
+      def atom;          combine_adjacent(*ATOM_TOKENS)          end
+      def astring_chars; combine_adjacent(*ASTRING_CHARS_TOKENS) end
+      def tag;           combine_adjacent(*TAG_TOKENS)           end
 
       # the #accept version of #atom
       def atom?; -combine_adjacent(*ATOM_TOKENS) if lookahead?(*ATOM_TOKENS) end
@@ -336,11 +354,6 @@ def case_insensitive__atom?
         -combine_adjacent(*ATOM_TOKENS).upcase if lookahead?(*ATOM_TOKENS)
       end
 
-      # TODO: handle astring_chars entirely inside the lexer
-      def astring_chars
-        combine_adjacent(*ASTRING_CHARS_TOKENS)
-      end
-
       #   astring         = 1*ASTRING-CHAR / string
       def astring
         lookahead?(*ASTRING_CHARS_TOKENS) ? astring_chars : string
@@ -357,6 +370,17 @@ def label(word)
         parse_error("unexpected atom %p, expected %p instead", val, word)
       end
 
+      # expects "OK" or "NO" or "BAD" and raises InvalidResponseError on failure
+      def resp_cond_state__name
+        if RESP_COND_STATES.include?(actual = tagged_ext_label)
+          actual
+        else
+          raise InvalidResponseError, "bad response type %p, expected %s" % [
+            actual, RESP_COND_STATES.join(" or ")
+          ]
+        end
+      end
+
       #   nstring         = string / nil
       def nstring
         NIL? ? nil : string
@@ -452,13 +476,18 @@ def response_untagged
         end
       end
 
+      # RFC3501 & RFC9051:
+      #   response-tagged = tag SP resp-cond-state CRLF
+      #
+      #   resp-cond-state = ("OK" / "NO" / "BAD") SP resp-text
+      #                       ; Status condition
+      #
+      #   tag             = 1*<any ASTRING-CHAR except "+">
       def response_tagged
-        tag = astring_chars
-        match(T_SPACE)
-        token = match(T_ATOM)
-        name = token.value.upcase
-        match(T_SPACE)
-        return TaggedResponse.new(tag, name, resp_text, @str)
+        tag  = tag();                 SP!
+        name = resp_cond_state__name; SP!
+        data = resp_text
+        TaggedResponse.new(tag, name, data, @str)
       end
 
       def response_cond

test/net/imap/test_imap.rb

@@ -147,9 +147,10 @@ def test_starttls_stripping
       imap = nil
       starttls_stripping_test do |port|
         imap = Net::IMAP.new("localhost", :port => port)
-        assert_raise(Net::IMAP::UnknownResponseError) do
+        assert_raise(Net::IMAP::InvalidResponseError) do
           imap.starttls(:ca_file => CA_FILE)
         end
+        assert imap.disconnected?
         imap
       end
       assert_equal false, imap.tls_verified?