Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect escaped quotation marks in readQuotedString #5

Merged
merged 5 commits into from
Mar 25, 2019

Conversation

mathisgerdes
Copy link
Contributor

@mathisgerdes mathisgerdes commented Mar 21, 2019

Within quoted string quotation marks may appear as ...\"..., which should not terminate the quote.
I observed this multiple times in fetch responses from gmail, leading to an error in the previous version.

Within quoted string quotation marks may appear as ...\"... which
should not terminate the quote.
@mathisgerdes
Copy link
Contributor Author

mathisgerdes commented Mar 21, 2019

According to RFC 2683 the two cases to be handled are \" and \\.

@mathisgerdes
Copy link
Contributor Author

I am not sure how you would like to handle the case of \ followed by something other than \ or ". Currently the chars are added verbatim (i.e. treating \<not \ or "> as \\<not \ or ">); I don't know if you'd rather throw an error.

@michaelspiss
Copy link
Owner

Thank you for reporting this, I cannot remember anything like that from reading the rfc. Could you please open an issue for this so we can discuss it over there (preferably with logs)? I am currently on vacation, so I won't be able to confirm anything until I get back home, but we should at least be able to (1) get the logs ready for me to reproduce the error and (2) maybe already provide a fix that only needs to be merged

@mathisgerdes
Copy link
Contributor Author

mathisgerdes commented Mar 22, 2019

Please excuse my potential confusion at this point, but is this pull request not a way to provide a fix that only needs to be merged? I'm not quite sure which logs you refer to, besides maybe the explicit error which occurs (the code continues to scan part of the subject line and fails with an unexpected imap word).

To reproduce the error try to fetch the envelop of an email that contains a quotation mark in the subject line.

As mentioned in a previous comment, you can read about the escaped characters here:
https://tools.ietf.org/html/rfc2683#section-3.4.2

@mathisgerdes mathisgerdes changed the base branch from master to develop March 22, 2019 22:40
@michaelspiss
Copy link
Owner

I am so sorry for the confusion, I missed the comment where you mentioned rfc 2683 on my mobile - thus the strange response. With logs I meant this package's logs which can be viewed by calling printImapClientDebugLog(), but this is obsolete due to the nature of this issue.

The only thing missing would be to add one or two tests for this case so it won't be overlooked in future versions (in test/imap_buffer_test.dart), I will merge this pull request as soon as I'm back home.

Again, I am terribly sorry for the confusion caused.

@michaelspiss
Copy link
Owner

After a lot of consideration I think it's best to throw a SyntaxErrorException if the character to escape is neither a backslash nor a quotation mark. Whilst every character is escapable, most simply don't have any meaning to the protocol, plus some other characters have special meanings when escaped (\t for tab, etc.) which would produce wrong results.

Add tests for escaped characters in quoted string and throw
a SyntaxErrorException if an escape sequence besides backslash or
quotation mark appears.
This is to avoid confusion regarding the new escaped characters and single - non-escaped - characters
@michaelspiss
Copy link
Owner

Thank you for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants