Skip to content

Commit

Permalink
🥅 Return UnparsedData for unhandled response-data
Browse files Browse the repository at this point in the history
Previously, unhandled `response-data` would attempt to parse to the end
of the string using `text`.  This would fail if the string contained
either literals or binary data.  The new `unparsed_response` approach
simply grabs all remaining bytes (except for the final CRLF).  Rather
than simply return string data, the response's data uses a new
`UnparsedData` struct, to unambiguously signal that the result may
change if a future release starts parsing the string.

The same approach has been copied over to `numeric_response`, allowing
unhandled numeric responses to be handled more robustly as well.

`IgnoredResponse` was converted to a subclass of `UntaggedResponse`, and
assigns any remaining unparsed text to an `UnparsedData` struct.  Other
than the subclass, it is now treated like any other unknown/unparsed
response type.  +NOOP+ still returns IgnoredResponse.
  • Loading branch information
nevans committed Oct 21, 2023
1 parent 724fe71 commit a054a09
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 25 deletions.
29 changes: 22 additions & 7 deletions lib/net/imap/response_data.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,17 +55,32 @@ class UntaggedResponse < Struct.new(:name, :data, :raw_data)

# Net::IMAP::IgnoredResponse represents intentionally ignored responses.
#
# This includes untagged response "NOOP" sent by eg. Zimbra to avoid some
# clients to close the connection.
# This includes untagged response "NOOP" sent by eg. Zimbra to avoid
# some clients to close the connection.
#
# It matches no IMAP standard.
#
class IgnoredResponse < Struct.new(:raw_data)
class IgnoredResponse < UntaggedResponse
end

# Net::IMAP::UnparsedData represents data for unknown or unhandled
# response types. UnparsedData is an intentionally unstable API: where
# it is returned, future releases may return a different (incompatible)
# object without deprecation or warning.
class UnparsedData < Struct.new(:number, :unparsed_data)
##
# method: raw_data
# :call-seq: raw_data -> string
# method: number
# :call-seq: number -> integer
#
# The raw response data.
# Returns a numeric response data prefix, when available.
#
# Many response types are prefixed with seqno or uid (for message
# data) or a message count (for mailbox data).

##
# method: unparsed_data
# :call-seq: string -> string
#
# The unparsed data, not including #number or UntaggedResponse#name.
end

# Net::IMAP::TaggedResponse represents tagged responses.
Expand Down
36 changes: 21 additions & 15 deletions lib/net/imap/response_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -476,13 +476,29 @@ def response_data
when /\A(?:ENABLED)\z/ni
return enable_data
else
return text_response
return unparsed_response
end
else
parse_error("unexpected token %s", token.symbol)
end
end

def unparsed_response(klass = UntaggedResponse)
num = number?; SP?
type = tagged_ext_label; SP?
text = remaining_unparsed
data = UnparsedData.new(num, text) if num || text
klass.new(type, data, @str)
end

# reads all the way up until CRLF
def remaining_unparsed
str = @str[@pos...-2] and @pos += str.bytesize
str&.empty? ? nil : str
end

def ignored_response; unparsed_response(IgnoredResponse) end

# RFC3501 & RFC9051:
# response-tagged = tag SP resp-cond-state CRLF
#
Expand Down Expand Up @@ -517,6 +533,10 @@ def numeric_response
match(T_SPACE)
data = FetchData.new(n, msg_att(n))
return UntaggedResponse.new(name, data, @str)
else
klass = name == "NOOP" ? IgnoredResponse : UntaggedResponse
SP?; txt = remaining_unparsed
klass.new(name, UnparsedData.new(n, txt), @str)
end
end

Expand Down Expand Up @@ -976,20 +996,6 @@ def modseq_data
return name, modseq
end

def ignored_response
while lookahead.symbol != T_CRLF
shift_token
end
return IgnoredResponse.new(@str)
end

def text_response
token = match(T_ATOM)
name = token.value.upcase
match(T_SPACE)
return UntaggedResponse.new(name, text)
end

def flags_response
token = match(T_ATOM)
name = token.value.upcase
Expand Down
20 changes: 17 additions & 3 deletions test/net/imap/fixtures/response_parser/quirky_behaviors.yml
Original file line number Diff line number Diff line change
@@ -1,9 +1,23 @@
---
:tests:
test_invalid_noop_response_is_ignored:
:comment: |
This should probably use UntaggedResponse, perhaps with an
UnparsedResponseData object for its #data.
:response: "* NOOP\r\n"
:expected: !ruby/struct:Net::IMAP::IgnoredResponse
name: "NOOP"
raw_data: "* NOOP\r\n"

test_invalid_noop_response_with_unparseable_data:
:response: "* NOOP froopy snood\r\n"
:expected: !ruby/struct:Net::IMAP::IgnoredResponse
name: "NOOP"
data: !ruby/struct:Net::IMAP::UnparsedData
unparsed_data: "froopy snood"
raw_data: "* NOOP froopy snood\r\n"

test_invalid_noop_response_with_numeric_prefix:
:response: "* 99 NOOP\r\n"
:expected: !ruby/struct:Net::IMAP::IgnoredResponse
name: "NOOP"
data: !ruby/struct:Net::IMAP::UnparsedData
number: 99
raw_data: "* 99 NOOP\r\n"

0 comments on commit a054a09

Please sign in to comment.