Skip to content

Commit 191d716

Browse files
nevansnevans
committed
🥅 Successfully parse invalid response code data
When the parser encounters a recoverable error in `resp-text-code`, it now returns `InvalidParseData` to represent the data that we've skipped over. `InvalidParseData` can be used for similar recoverable parse errors in the future (for example, many servers respond with invalid `BODYSTRUCTURE` or incorrectly escaped quoted strings). The specific example I have encountered the most is when Microsoft's IMAP servers send an invalid COPYUID response code. Although it is invalid for `resp-code-copy`, it's still a valid `resp-text-code` because it does match `atom [SP 1*<any TEXT-CHAR except "]">]`. This creates some minor differences for invalid `resp-text-code` data: * <= v0.6.2: raises ResponseParseError (this is a bug). * == v0.6.3: returns ResponseText with no ResponseCode (also a bug). * >= v0.6.4: returns ResponseText with code with InvalidParseData. Although this is a bugfix, it has a minor incompatibility for response handlers which assume that a particular `ResponseCode#name` always results in the same type of `ResponseCode#data`. ```ruby # It was previously safe to assume the class of #data, based on #name: imap.add_response_handler do |resp| if resp in {data: {code: {name: "COPYUID", data: opyuid}}} copyuid => Net::IMAP::CopyUIDData end end # With this change, ResponseCode#data could also be InvalidParseData imap.add_response_handler do |resp| if resp in {data: {code: {name: "COPYUID", data: copyuid}}} copyuid => Net::IMAP::CopyUIDData | Net::IMAP::InvalidParseData end end ``` Prior to v0.6.3, these responses would raise a ResponseParseError and the response handler would not have been called.
1 parent 728fb88 commit 191d716

3 files changed

Lines changed: 236 additions & 6 deletions

File tree

‎lib/net/imap/response_data.rb‎

Lines changed: 73 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,8 @@ class IgnoredResponse < UntaggedResponse
8585
# data: Net::IMAP::UnparsedData(unparsed_data: "can't parse this"),
8686
# )
8787
#
88-
# See also: UnparsedNumericResponseData, ExtensionData, IgnoredResponse
88+
# See also: UnparsedNumericResponseData, ExtensionData, IgnoredResponse,
89+
# InvalidParseData.
8990
class UnparsedData < Struct.new(:unparsed_data)
9091
##
9192
# method: unparsed_data
@@ -94,6 +95,65 @@ class UnparsedData < Struct.new(:unparsed_data)
9495
# The unparsed data
9596
end
9697

98+
# **Note:** This represents an intentionally _unstable_ API. Where
99+
# instances of this class are returned, future releases may return a
100+
# different (incompatible) object <em>without deprecation or warning</em>.
101+
#
102+
# When the response parser encounters a recoverable error,
103+
# Net::IMAP::InvalidParseData represents that portion of the response which
104+
# could not be parsed, allowing the parser to parse the remainder of the
105+
# response. InvalidParseData is always associated with a ResponseParseError
106+
# which has been rescued.
107+
#
108+
# This could be caused by a malformed server response, by a bug in
109+
# Net::IMAP::ResponseParser, or by an unsupported extension to the response
110+
# syntax. For example, if a server supports +UIDPLUS+, but sends an invalid
111+
# +COPYUID+ response code:
112+
#
113+
# parser = Net::IMAP::ResponseParser.new
114+
# parsed = parser.parse "* OK [COPYUID 701 ] copied one message\r\n"
115+
# parsed => {
116+
# data: Net::IMAP::ResponseText(
117+
# code: Net::IMAP::ResponseCode(
118+
# name: "COPYUID",
119+
# data: Net::IMAP::InvalidParseData(
120+
# parse_error: Net::IMAP::ResponseParseError,
121+
# unparsed_data: "701 ",
122+
# parsed_data: nil,
123+
# )
124+
# )
125+
# )
126+
# }
127+
# data
128+
# #=> #<data Net::IMAP::InvalidParseData
129+
# # parse_error=#<Net::IMAP::ResponseParseError: ...>,
130+
# # unparsed_data="701 ">
131+
#
132+
# In this example, <tt>[COPYUID 701 ]</tt> _does_ use valid syntax for a
133+
# generic ResponseCode, but uses _invalid_ syntax for a +COPYUID+ response
134+
# code.
135+
#
136+
# See also: UnparsedData, ExtensionData
137+
class InvalidParseData < Data.define(:parse_error, :unparsed_data, :parsed_data)
138+
##
139+
# method: parse_error
140+
# :call-seq: parse_error -> ResponseParseError
141+
#
142+
# Returns the rescued ResponseParseError.
143+
144+
##
145+
# method: unparsed_data
146+
# :call-seq: unparsed_data -> string
147+
#
148+
# Returns the raw string which was skipped over by the parser.
149+
150+
##
151+
# method: parsed_data
152+
#
153+
# May return a partial parse result for unparsed_data, which had already
154+
# been parsed before the parse_error.
155+
end
156+
97157
# **Note:** This represents an intentionally _unstable_ API. Where
98158
# instances of this class are returned, future releases may return a
99159
# different (incompatible) object <em>without deprecation or warning</em>.
@@ -111,6 +171,7 @@ class UnparsedData < Struct.new(:unparsed_data)
111171
# ),
112172
# )
113173
#
174+
# See also: UnparsedData, ExtensionData, IgnoredResponse, InvalidParseData
114175
class UnparsedNumericResponseData < Struct.new(:number, :unparsed_data)
115176
##
116177
# method: number
@@ -341,9 +402,10 @@ class ResponseText < Struct.new(:code, :text)
341402
#
342403
# Response codes are backwards compatible: Servers are allowed to send new
343404
# response codes even if the client has not enabled the extension that
344-
# defines them. When Net::IMAP does not know how to parse response
345-
# code text, #data returns the unparsed string.
346-
#
405+
# defines them. When ResponseParser does not know how to parse the response
406+
# code data, #data may return the unparsed string, ExtensionData, or
407+
# UnparsedData. When ResponseParser attempts but fails to parse the
408+
# response code data, #data returns InvalidParseData.
347409
class ResponseCode < Struct.new(:name, :data)
348410
##
349411
# method: name
@@ -358,8 +420,13 @@ class ResponseCode < Struct.new(:name, :data)
358420
#
359421
# Returns the parsed response code data, e.g: an array of capabilities
360422
# strings, an array of character set strings, a list of permanent flags,
361-
# an Integer, etc. The response #code determines what form the response
362-
# code data can take.
423+
# an Integer, etc. The response #name determines what form the response
424+
# code #data can take.
425+
#
426+
# When ResponseParser does not know how to parse the response code data,
427+
# #data may return the unparsed string, ExtensionData, or UnparsedData.
428+
# When ResponseParser attempts but fails to parse the response code data,
429+
# #data returns InvalidParseData.
363430
end
364431

365432
# MailboxList represents the data of an untagged +LIST+ response, for a

‎lib/net/imap/response_parser.rb‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1961,6 +1961,7 @@ def resp_text
19611961
# resp-text-code =/ "UIDREQUIRED"
19621962
def resp_text_code
19631963
name = resp_text_code__name
1964+
state = current_state
19641965
data =
19651966
case name
19661967
when "CAPABILITY" then resp_code__capability
@@ -1983,8 +1984,18 @@ def resp_text_code
19831984
when "MAILBOXID" then SP!; parens__objectid # RFC8474: OBJECTID
19841985
when "UIDREQUIRED" then # RFC9586: UIDONLY
19851986
else
1987+
state = nil # don't backtrack
19861988
SP? and text_chars_except_rbra
19871989
end
1990+
peek_rbra? or
1991+
parse_error("expected resp-text-code %p to be complete", name)
1992+
ResponseCode.new(name, data)
1993+
rescue Net::IMAP::ResponseParseError => parse_error
1994+
raise unless state
1995+
raise if parse_error.message.include?("uid-set")
1996+
restore_state state
1997+
unparsed_data = SP? && text_chars_except_rbra
1998+
data = InvalidParseData[parse_error:, unparsed_data:, parsed_data: data]
19881999
ResponseCode.new(name, data)
19892000
end
19902001

‎test/net/imap/fixtures/response_parser/quirky_behaviors.yml‎

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,158 @@
5757
MailboxBE=XXXXXXXXXXXXX.EURXXXX.PROD.OUTLOOK.COM Service=Imap4] AUTHENTICATE
5858
completed.\r\n"
5959

60+
"Outlook.com and Microsoft 365 can send an invalid COPYUID response code":
61+
comment: |
62+
Although this is a buggy COPYUID response from the server, it's still a
63+
valid *generic* `resp-text-code`. We should always successfully parse
64+
`resp-text-code` when it begins with a valid `atom SP`, even if that means
65+
using UnparsedData or some other similar class to wrap the invalid or
66+
unparsable payload that follows.
67+
:response: "* OK [COPYUID 701 ]\r\n"
68+
:expected: !ruby/struct:Net::IMAP::UntaggedResponse
69+
name: OK
70+
data: !ruby/struct:Net::IMAP::ResponseText
71+
code: !ruby/struct:Net::IMAP::ResponseCode
72+
name: COPYUID
73+
data: !ruby/data:Net::IMAP::InvalidParseData
74+
parse_error: !ruby/exception:Net::IMAP::ResponseParseError
75+
message: unexpected token SPACE (expected ATOM or NUMBER or STAR)
76+
parser_class: !ruby/class 'Net::IMAP::ResponseParser'
77+
string: "* OK [COPYUID 701 ]\r\n"
78+
pos: 19
79+
lex_state: :EXPR_BEG
80+
token: !ruby/struct:Net::IMAP::ResponseParser::Token
81+
symbol: :SPACE
82+
value: " "
83+
backtrace:
84+
- "lib/net/imap/response_parser/parser_utils.rb:218:in `parse_error'"
85+
- "lib/net/imap/response_parser/parser_utils.rb:127:in `combine_adjacent'"
86+
- "lib/net/imap/response_parser.rb:499:in `sequence_set'"
87+
- "lib/net/imap/response_parser.rb:2180:in `uid_set'"
88+
- "lib/net/imap/response_parser.rb:2034:in `resp_code_copy__data'"
89+
- "lib/net/imap/response_parser.rb:1973:in `resp_text_code'"
90+
- "lib/net/imap/response_parser.rb:1894:in `resp_text'"
91+
- "lib/net/imap/response_parser.rb:820:in `resp_cond_state'"
92+
- "lib/net/imap/response_parser.rb:824:in `resp_cond_state__untagged'"
93+
- "lib/net/imap/response_parser.rb:740:in `response_data'"
94+
- "lib/net/imap/response_parser.rb:687:in `response'"
95+
- "lib/net/imap/response_parser.rb:40:in `parse'"
96+
- "test/net/imap/net_imap_test_helpers.rb ... ignoring 3 frames"
97+
- "/gems/test-unit-3.7.3/lib/test/unit/... ignoring 42 frames"
98+
unparsed_data: '701 '
99+
text: ""
100+
raw_data: "* OK [COPYUID 701 ]\r\n"
101+
102+
"Extra resp-text-code payload":
103+
comment: |
104+
`resp-text-code` should parse when its payload continues unexpectedly.
105+
:response: "* OK [COPYUID 1 1:10 101:110 extra junk] bad\r\n"
106+
:expected: !ruby/struct:Net::IMAP::UntaggedResponse
107+
name: OK
108+
data: !ruby/struct:Net::IMAP::ResponseText
109+
code: !ruby/struct:Net::IMAP::ResponseCode
110+
name: COPYUID
111+
data: !ruby/data:Net::IMAP::InvalidParseData
112+
parse_error: !ruby/exception:Net::IMAP::ResponseParseError
113+
message: expected resp-text-code "COPYUID" to be complete
114+
backtrace:
115+
- "lib/net/imap/response_parser/parser_utils.rb:218:in 'Net::IMAP::ResponseParser::ParserUtils#parse_error'"
116+
- "lib/net/imap/response_parser.rb:1981:in 'Net::IMAP::ResponseParser#resp_text_code'"
117+
- "lib/net/imap/response_parser.rb:1894:in 'Net::IMAP::ResponseParser#resp_text'"
118+
- "lib/net/imap/response_parser.rb:820:in 'Net::IMAP::ResponseParser#resp_cond_state'"
119+
- "lib/net/imap/response_parser.rb:824:in 'Net::IMAP::ResponseParser#resp_cond_state__untagged'"
120+
- "lib/net/imap/response_parser.rb:740:in 'Net::IMAP::ResponseParser#response_data'"
121+
- "lib/net/imap/response_parser.rb:687:in 'Net::IMAP::ResponseParser#response'"
122+
- "lib/net/imap/response_parser.rb:40:in 'Net::IMAP::ResponseParser#parse'"
123+
- "test/net/imap/net_imap_test_helpers.rb ... ignoring 3 frames"
124+
- "/gems/test-unit-3.7.3/lib/test/unit/... ignoring 42 frames"
125+
parser_class: !ruby/class 'Net::IMAP::ResponseParser'
126+
string: "* OK [COPYUID 1 1:10 101:110 extra junk] bad\r\n"
127+
pos: 29
128+
lex_state: :EXPR_BEG
129+
token: !ruby/struct:Net::IMAP::ResponseParser::Token
130+
symbol: :SPACE
131+
value: " "
132+
unparsed_data: 1 1:10 101:110 extra junk
133+
parsed_data: !ruby/data:Net::IMAP::CopyUIDData
134+
uidvalidity: 1
135+
source_uids: !ruby/object:Net::IMAP::SequenceSet
136+
string: '1:10'
137+
assigned_uids: !ruby/object:Net::IMAP::SequenceSet
138+
string: 101:110
139+
text: bad
140+
raw_data: "* OK [COPYUID 1 1:10 101:110 extra junk] bad\r\n"
141+
142+
"Missing resp-text-code payload":
143+
comment: |
144+
`resp-text-code` should parse even when its expected payload is missing.
145+
:response: "* OK [COPYUID] missing\r\n"
146+
:expected: !ruby/struct:Net::IMAP::UntaggedResponse
147+
name: OK
148+
data: !ruby/struct:Net::IMAP::ResponseText
149+
code: !ruby/struct:Net::IMAP::ResponseCode
150+
name: COPYUID
151+
data: !ruby/data:Net::IMAP::InvalidParseData
152+
parse_error: !ruby/exception:Net::IMAP::ResponseParseError
153+
message: unexpected RBRA (expected " ")
154+
backtrace:
155+
- "lib/net/imap/response_parser/parser_utils.rb:218:in 'Net::IMAP::ResponseParser::ParserUtils#parse_error'"
156+
- "lib/net/imap/response_parser/parser_utils.rb:54:in 'Net::IMAP::ResponseParser#SP!'"
157+
- "lib/net/imap/response_parser.rb:1973:in 'Net::IMAP::ResponseParser#resp_text_code'"
158+
- "lib/net/imap/response_parser.rb:1894:in 'Net::IMAP::ResponseParser#resp_text'"
159+
- "lib/net/imap/response_parser.rb:820:in 'Net::IMAP::ResponseParser#resp_cond_state'"
160+
- "lib/net/imap/response_parser.rb:824:in 'Net::IMAP::ResponseParser#resp_cond_state__untagged'"
161+
- "lib/net/imap/response_parser.rb:740:in 'Net::IMAP::ResponseParser#response_data'"
162+
- "lib/net/imap/response_parser.rb:687:in 'Net::IMAP::ResponseParser#response'"
163+
- "lib/net/imap/response_parser.rb:40:in 'Net::IMAP::ResponseParser#parse'"
164+
- "test/net/imap/net_imap_test_helpers.rb ... ignoring 3 frames"
165+
- "/gems/test-unit-3.7.3/lib/test/unit/... ignoring 42 frames"
166+
parser_class: !ruby/class 'Net::IMAP::ResponseParser'
167+
string: "* OK [COPYUID] missing\r\n"
168+
pos: 14
169+
lex_state: :EXPR_BEG
170+
token: !ruby/struct:Net::IMAP::ResponseParser::Token
171+
symbol: :RBRA
172+
value: "]"
173+
unparsed_data:
174+
parsed_data:
175+
text: missing
176+
raw_data: "* OK [COPYUID] missing\r\n"
177+
178+
"Unexpected resp-text-code payload":
179+
comment: |
180+
`resp-text-code` should parse even when a payload is unexpectedly present.
181+
:response: "* OK [CLOSED shouldn't be anything here] mailbox closed\r\n"
182+
:expected: !ruby/struct:Net::IMAP::UntaggedResponse
183+
name: OK
184+
data: !ruby/struct:Net::IMAP::ResponseText
185+
code: !ruby/struct:Net::IMAP::ResponseCode
186+
name: CLOSED
187+
data: !ruby/data:Net::IMAP::InvalidParseData
188+
parse_error: !ruby/exception:Net::IMAP::ResponseParseError
189+
message: expected resp-text-code "CLOSED" to be complete
190+
backtrace:
191+
- "lib/net/imap/response_parser/parser_utils.rb:218:in 'Net::IMAP::ResponseParser::ParserUtils#parse_error'"
192+
- "lib/net/imap/response_parser.rb:1981:in 'Net::IMAP::ResponseParser#resp_text_code'"
193+
- "lib/net/imap/response_parser.rb:1894:in 'Net::IMAP::ResponseParser#resp_text'"
194+
- "lib/net/imap/response_parser.rb:820:in 'Net::IMAP::ResponseParser#resp_cond_state'"
195+
- "lib/net/imap/response_parser.rb:824:in 'Net::IMAP::ResponseParser#resp_cond_state__untagged'"
196+
- "lib/net/imap/response_parser.rb:740:in 'Net::IMAP::ResponseParser#response_data'"
197+
- "lib/net/imap/response_parser.rb:687:in 'Net::IMAP::ResponseParser#response'"
198+
- "lib/net/imap/response_parser.rb:40:in 'Net::IMAP::ResponseParser#parse'"
199+
- "test/net/imap/net_imap_test_helpers.rb ... ignoring 3 frames"
200+
- "/gems/test-unit-3.7.3/lib/test/unit/... ignoring 42 frames"
201+
parser_class: !ruby/class 'Net::IMAP::ResponseParser'
202+
string: "* OK [CLOSED shouldn't be anything here] mailbox closed\r\n"
203+
pos: 13
204+
lex_state: :EXPR_BEG
205+
token: !ruby/struct:Net::IMAP::ResponseParser::Token
206+
symbol: :SPACE
207+
value: " "
208+
unparsed_data: shouldn't be anything here
209+
text: mailbox closed
210+
raw_data: "* OK [CLOSED shouldn't be anything here] mailbox closed\r\n"
211+
60212
outlook.com puts an extra SP in ENVELOPE address lists:
61213
comment: |
62214
An annoying bug from outlook.com. They've had the bug for years, and

0 commit comments

Comments
 (0)