Skip to content

Commit 7f0e115

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 73121cf commit 7f0e115

File tree

3 files changed

+234
-6
lines changed

3 files changed

+234
-6
lines changed

‎lib/net/imap/response_data.rb‎

Lines changed: 69 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,61 @@ 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+
#
128+
# In this example, although <tt>[COPYUID 701 ]</tt> uses valid syntax for a
129+
# _generic_ ResponseCode, it is _invalid_ syntax for a +COPYUID+ response
130+
# code.
131+
#
132+
# See also: UnparsedData, ExtensionData
133+
class InvalidParseData < Data.define(:parse_error, :unparsed_data, :parsed_data)
134+
##
135+
# method: parse_error
136+
# :call-seq: parse_error -> ResponseParseError
137+
#
138+
# Returns the rescued ResponseParseError.
139+
140+
##
141+
# method: unparsed_data
142+
# :call-seq: unparsed_data -> string
143+
#
144+
# Returns the raw string which was skipped over by the parser.
145+
146+
##
147+
# method: parsed_data
148+
#
149+
# May return a partial parse result for unparsed_data, which had already
150+
# been parsed before the parse_error.
151+
end
152+
97153
# **Note:** This represents an intentionally _unstable_ API. Where
98154
# instances of this class are returned, future releases may return a
99155
# different (incompatible) object <em>without deprecation or warning</em>.
@@ -111,6 +167,7 @@ class UnparsedData < Struct.new(:unparsed_data)
111167
# ),
112168
# )
113169
#
170+
# See also: UnparsedData, ExtensionData, IgnoredResponse, InvalidParseData
114171
class UnparsedNumericResponseData < Struct.new(:number, :unparsed_data)
115172
##
116173
# method: number
@@ -341,9 +398,10 @@ class ResponseText < Struct.new(:code, :text)
341398
#
342399
# Response codes are backwards compatible: Servers are allowed to send new
343400
# 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-
#
401+
# defines them. When ResponseParser does not know how to parse the response
402+
# code data, #data may return the unparsed string, ExtensionData, or
403+
# UnparsedData. When ResponseParser attempts but fails to parse the
404+
# response code data, #data returns InvalidParseData.
347405
class ResponseCode < Struct.new(:name, :data)
348406
##
349407
# method: name
@@ -358,8 +416,13 @@ class ResponseCode < Struct.new(:name, :data)
358416
#
359417
# Returns the parsed response code data, e.g: an array of capabilities
360418
# 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.
419+
# an Integer, etc. The response #name determines what form the response
420+
# code #data can take.
421+
#
422+
# When ResponseParser does not know how to parse the response code data,
423+
# #data may return the unparsed string, ExtensionData, or UnparsedData.
424+
# When ResponseParser attempts but fails to parse the response code data,
425+
# #data returns InvalidParseData.
363426
end
364427

365428
# 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: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,160 @@
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+
parsed_data:
100+
text: ""
101+
raw_data: "* OK [COPYUID 701 ]\r\n"
102+
103+
"Extra resp-text-code payload":
104+
comment: |
105+
`resp-text-code` should parse when its payload continues unexpectedly.
106+
:response: "* OK [COPYUID 1 1:10 101:110 extra junk] bad\r\n"
107+
:expected: !ruby/struct:Net::IMAP::UntaggedResponse
108+
name: OK
109+
data: !ruby/struct:Net::IMAP::ResponseText
110+
code: !ruby/struct:Net::IMAP::ResponseCode
111+
name: COPYUID
112+
data: !ruby/data:Net::IMAP::InvalidParseData
113+
parse_error: !ruby/exception:Net::IMAP::ResponseParseError
114+
message: expected resp-text-code "COPYUID" to be complete
115+
backtrace:
116+
- "lib/net/imap/response_parser/parser_utils.rb:218:in 'Net::IMAP::ResponseParser::ParserUtils#parse_error'"
117+
- "lib/net/imap/response_parser.rb:1981:in 'Net::IMAP::ResponseParser#resp_text_code'"
118+
- "lib/net/imap/response_parser.rb:1894:in 'Net::IMAP::ResponseParser#resp_text'"
119+
- "lib/net/imap/response_parser.rb:820:in 'Net::IMAP::ResponseParser#resp_cond_state'"
120+
- "lib/net/imap/response_parser.rb:824:in 'Net::IMAP::ResponseParser#resp_cond_state__untagged'"
121+
- "lib/net/imap/response_parser.rb:740:in 'Net::IMAP::ResponseParser#response_data'"
122+
- "lib/net/imap/response_parser.rb:687:in 'Net::IMAP::ResponseParser#response'"
123+
- "lib/net/imap/response_parser.rb:40:in 'Net::IMAP::ResponseParser#parse'"
124+
- "test/net/imap/net_imap_test_helpers.rb ... ignoring 3 frames"
125+
- "/gems/test-unit-3.7.3/lib/test/unit/... ignoring 42 frames"
126+
parser_class: !ruby/class 'Net::IMAP::ResponseParser'
127+
string: "* OK [COPYUID 1 1:10 101:110 extra junk] bad\r\n"
128+
pos: 29
129+
lex_state: :EXPR_BEG
130+
token: !ruby/struct:Net::IMAP::ResponseParser::Token
131+
symbol: :SPACE
132+
value: " "
133+
unparsed_data: 1 1:10 101:110 extra junk
134+
parsed_data: !ruby/data:Net::IMAP::CopyUIDData
135+
uidvalidity: 1
136+
source_uids: !ruby/object:Net::IMAP::SequenceSet
137+
string: '1:10'
138+
assigned_uids: !ruby/object:Net::IMAP::SequenceSet
139+
string: 101:110
140+
text: bad
141+
raw_data: "* OK [COPYUID 1 1:10 101:110 extra junk] bad\r\n"
142+
143+
"Missing resp-text-code payload":
144+
comment: |
145+
`resp-text-code` should parse even when its expected payload is missing.
146+
:response: "* OK [COPYUID] missing\r\n"
147+
:expected: !ruby/struct:Net::IMAP::UntaggedResponse
148+
name: OK
149+
data: !ruby/struct:Net::IMAP::ResponseText
150+
code: !ruby/struct:Net::IMAP::ResponseCode
151+
name: COPYUID
152+
data: !ruby/data:Net::IMAP::InvalidParseData
153+
parse_error: !ruby/exception:Net::IMAP::ResponseParseError
154+
message: unexpected RBRA (expected " ")
155+
backtrace:
156+
- "lib/net/imap/response_parser/parser_utils.rb:218:in 'Net::IMAP::ResponseParser::ParserUtils#parse_error'"
157+
- "lib/net/imap/response_parser/parser_utils.rb:54:in 'Net::IMAP::ResponseParser#SP!'"
158+
- "lib/net/imap/response_parser.rb:1973:in 'Net::IMAP::ResponseParser#resp_text_code'"
159+
- "lib/net/imap/response_parser.rb:1894:in 'Net::IMAP::ResponseParser#resp_text'"
160+
- "lib/net/imap/response_parser.rb:820:in 'Net::IMAP::ResponseParser#resp_cond_state'"
161+
- "lib/net/imap/response_parser.rb:824:in 'Net::IMAP::ResponseParser#resp_cond_state__untagged'"
162+
- "lib/net/imap/response_parser.rb:740:in 'Net::IMAP::ResponseParser#response_data'"
163+
- "lib/net/imap/response_parser.rb:687:in 'Net::IMAP::ResponseParser#response'"
164+
- "lib/net/imap/response_parser.rb:40:in 'Net::IMAP::ResponseParser#parse'"
165+
- "test/net/imap/net_imap_test_helpers.rb ... ignoring 3 frames"
166+
- "/gems/test-unit-3.7.3/lib/test/unit/... ignoring 42 frames"
167+
parser_class: !ruby/class 'Net::IMAP::ResponseParser'
168+
string: "* OK [COPYUID] missing\r\n"
169+
pos: 14
170+
lex_state: :EXPR_BEG
171+
token: !ruby/struct:Net::IMAP::ResponseParser::Token
172+
symbol: :RBRA
173+
value: "]"
174+
unparsed_data:
175+
parsed_data:
176+
text: missing
177+
raw_data: "* OK [COPYUID] missing\r\n"
178+
179+
"Unexpected resp-text-code payload":
180+
comment: |
181+
`resp-text-code` should parse even when a payload is unexpectedly present.
182+
:response: "* OK [CLOSED shouldn't be anything here] mailbox closed\r\n"
183+
:expected: !ruby/struct:Net::IMAP::UntaggedResponse
184+
name: OK
185+
data: !ruby/struct:Net::IMAP::ResponseText
186+
code: !ruby/struct:Net::IMAP::ResponseCode
187+
name: CLOSED
188+
data: !ruby/data:Net::IMAP::InvalidParseData
189+
parse_error: !ruby/exception:Net::IMAP::ResponseParseError
190+
message: expected resp-text-code "CLOSED" to be complete
191+
backtrace:
192+
- "lib/net/imap/response_parser/parser_utils.rb:218:in 'Net::IMAP::ResponseParser::ParserUtils#parse_error'"
193+
- "lib/net/imap/response_parser.rb:1981:in 'Net::IMAP::ResponseParser#resp_text_code'"
194+
- "lib/net/imap/response_parser.rb:1894:in 'Net::IMAP::ResponseParser#resp_text'"
195+
- "lib/net/imap/response_parser.rb:820:in 'Net::IMAP::ResponseParser#resp_cond_state'"
196+
- "lib/net/imap/response_parser.rb:824:in 'Net::IMAP::ResponseParser#resp_cond_state__untagged'"
197+
- "lib/net/imap/response_parser.rb:740:in 'Net::IMAP::ResponseParser#response_data'"
198+
- "lib/net/imap/response_parser.rb:687:in 'Net::IMAP::ResponseParser#response'"
199+
- "lib/net/imap/response_parser.rb:40:in 'Net::IMAP::ResponseParser#parse'"
200+
- "test/net/imap/net_imap_test_helpers.rb ... ignoring 3 frames"
201+
- "/gems/test-unit-3.7.3/lib/test/unit/... ignoring 42 frames"
202+
parser_class: !ruby/class 'Net::IMAP::ResponseParser'
203+
string: "* OK [CLOSED shouldn't be anything here] mailbox closed\r\n"
204+
pos: 13
205+
lex_state: :EXPR_BEG
206+
token: !ruby/struct:Net::IMAP::ResponseParser::Token
207+
symbol: :SPACE
208+
value: " "
209+
unparsed_data: shouldn't be anything here
210+
parsed_data:
211+
text: mailbox closed
212+
raw_data: "* OK [CLOSED shouldn't be anything here] mailbox closed\r\n"
213+
60214
outlook.com puts an extra SP in ENVELOPE address lists:
61215
comment: |
62216
An annoying bug from outlook.com. They've had the bug for years, and

0 commit comments

Comments
 (0)