Skip to content

Commit 0b6129d

Browse files
nevansnevans
authored
🔀 Merge pull request #614 from ruby/parser/invalid-resp-text-code-data
🥅 Successfully parse invalid response code data
2 parents 4cb320f + 7f0e115 commit 0b6129d

File tree

3 files changed

+253
-8
lines changed

3 files changed

+253
-8
lines changed

‎lib/net/imap/response_data.rb‎

Lines changed: 88 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,18 @@ class IgnoredResponse < UntaggedResponse
7575
#
7676
# Net::IMAP::UnparsedData represents data for unknown response types or
7777
# unknown extensions to response types without a well-defined extension
78-
# grammar.
78+
# grammar. UnparsedData represents the portion of the response which the
79+
# parser has skipped over, without attempting to parse it.
7980
#
80-
# See also: UnparsedNumericResponseData, ExtensionData, IgnoredResponse
81+
# parser = Net::IMAP::ResponseParser.new
82+
# response = parser.parse "* X-UNKNOWN-TYPE can't parse this\r\n"
83+
# response => Net::IMAP::UntaggedResponse(
84+
# name: "X-UNKNOWN-TYPE",
85+
# data: Net::IMAP::UnparsedData(unparsed_data: "can't parse this"),
86+
# )
87+
#
88+
# See also: UnparsedNumericResponseData, ExtensionData, IgnoredResponse,
89+
# InvalidParseData.
8190
class UnparsedData < Struct.new(:unparsed_data)
8291
##
8392
# method: unparsed_data
@@ -86,14 +95,79 @@ class UnparsedData < Struct.new(:unparsed_data)
8695
# The unparsed data
8796
end
8897

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+
89153
# **Note:** This represents an intentionally _unstable_ API. Where
90154
# instances of this class are returned, future releases may return a
91155
# different (incompatible) object <em>without deprecation or warning</em>.
92156
#
93157
# Net::IMAP::UnparsedNumericResponseData represents data for unhandled
94158
# response types with a numeric prefix. See the documentation for #number.
95159
#
96-
# See also: UnparsedData, ExtensionData, IgnoredResponse
160+
# parser = Net::IMAP::ResponseParser.new
161+
# response = parser.parse "* 123 X-UNKNOWN-TYPE can't parse this\r\n"
162+
# response => Net::IMAP::UntaggedResponse(
163+
# name: "X-UNKNOWN-TYPE",
164+
# data: Net::IMAP::UnparsedNumericData(
165+
# number: 123,
166+
# unparsed_data: "can't parse this"
167+
# ),
168+
# )
169+
#
170+
# See also: UnparsedData, ExtensionData, IgnoredResponse, InvalidParseData
97171
class UnparsedNumericResponseData < Struct.new(:number, :unparsed_data)
98172
##
99173
# method: number
@@ -324,9 +398,10 @@ class ResponseText < Struct.new(:code, :text)
324398
#
325399
# Response codes are backwards compatible: Servers are allowed to send new
326400
# response codes even if the client has not enabled the extension that
327-
# defines them. When Net::IMAP does not know how to parse response
328-
# code text, #data returns the unparsed string.
329-
#
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.
330405
class ResponseCode < Struct.new(:name, :data)
331406
##
332407
# method: name
@@ -341,8 +416,13 @@ class ResponseCode < Struct.new(:name, :data)
341416
#
342417
# Returns the parsed response code data, e.g: an array of capabilities
343418
# strings, an array of character set strings, a list of permanent flags,
344-
# an Integer, etc. The response #code determines what form the response
345-
# 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.
346426
end
347427

348428
# 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)