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

Syntax error in BODYSTRUCTURE #485

Closed
breedbekkikker opened this issue Apr 5, 2017 · 24 comments

Comments

Projects
None yet
2 participants
@breedbekkikker
Copy link

commented Apr 5, 2017

Hi,

The following exception occurs when listing a folder:

Syntax error in BODYSTRUCTURE. Unexpected token: [qstring: " NIL NIL NIL {5}
8bit"]

The IMAP protocol logging:

C: N00000020 FETCH 1:* (UID FLAGS INTERNALDATE RFC822.SIZE ENVELOPE BODYSTRUCTURE)
S: * 1 FETCH (UID 1 FLAGS () INTERNALDATE " 5-Apr-2017 14:38:25 +0200" RFC822.SIZE 947 ENVELOPE ("Wed, 5 Apr 2017 14:38:25 +0200" "test" ((NIL NIL "test" "acme.com")) ((NIL NIL "test"" "acme.com")) ((NIL NIL "test" "acme.com")) NIL NIL NIL NIL "<54B6100D01175D77@acme.com>") BODYSTRUCTURE ("TEXT" "HTML"" NIL NIL NIL {5}
S: 8bit" 74 2))
S: N00000020 OK FETCH complete

The original mail contains the following headers (which are of course odd):

Content-Type: text/html"
Content-Transfer-Encoding: 8bit"

Is there are a solution possible for this within MailKit?

Thanks!

@jstedfast

This comment has been minimized.

Copy link
Owner

commented Apr 5, 2017

The problem is that your IMAP server is not escaping the double-quote character after the "html"

So it sends: ... "TEXT" "HTML"" ...

Not sure how I can work around that...

@breedbekkikker

This comment has been minimized.

Copy link
Author

commented Apr 5, 2017

Yes you are correct... I see similar exceptions with different messages with the same root cause.

@jstedfast

This comment has been minimized.

Copy link
Owner

commented Apr 5, 2017

Do they all end with a unescaped double-quote?

@breedbekkikker

This comment has been minimized.

Copy link
Author

commented Apr 5, 2017

Yes, but I found one that maybe a little different. The exception is:

Unexpected token in IMAP response: [qstring: ""]

The (scrubbed) protocol logging shows:

S: * 210 FETCH (UID 210 FLAGS (\Seen) INTERNALDATE " 6-Aug-2016 06:11:30 +0200" RFC822.SIZE 761409 ENVELOPE ("Mon, 6 Aug 2016 06:10:37 +0200" "test" (("test" NIL "test" "acme.com")) (("test" NIL "test" "acme.com")) (("test" NIL "test" "acme.com")) (("test" NIL "test" "acme.com")("test" NIL "test" "acme.com")) NIL "<5DBB50A5D3A54730AD4A54730AD4A54730AD440E@acme.com>" "<553B50A5A54730AD4A54730AD4A036427CA5474B3D53E6@acme.com>") BODYSTRUCTURE (("MOUNDARY="_006_5DBB50A5A54730AD4A54730AD4A54730AD4A54730AD42KOS_"" "OCTET-STREAM" ("name" "test.dat") NIL NIL "quoted-printable" 383137 NIL ("attachment" ("filename" "test.dat")))("MOUNDARY="_006_5DBB50A5D3ABEC4E85A03EAD527CA5474B3D0AF9E6EXMBXSVR02KOS_"" "OCTET-STREAM" ("name" "test.dat") NIL NIL "quoted-printable" 383137 NIL ("attachment" ("filename" "test.dat"))) "MIXED" ("boundary" "----=_NextPart_000_730AD4A547.730AD4A547F40")))

@jstedfast

This comment has been minimized.

Copy link
Owner

commented Apr 5, 2017

Yea, that's this: "MOUNDARY="_006_5DBB50A5D3ABEC4E85A03EAD527CA5474B3D0AF9E6EXMBXSVR02KOS_""

Which parses as:

  1. "MOUNDARY="
  2. _006_5DBB50A5D3ABEC4E85A03EAD527CA5474B3D0AF9E6EXMBXSVR02KOS_
  3. ""

It's failing because it expects token 3. to be '('

@jstedfast

This comment has been minimized.

Copy link
Owner

commented Apr 5, 2017

Could you try the following patch and see if this works for you?

diff --git a/MailKit/Net/Imap/ImapStream.cs b/MailKit/Net/Imap/ImapStream.cs
index a2cb3436..97721050 100644
--- a/MailKit/Net/Imap/ImapStream.cs
+++ b/MailKit/Net/Imap/ImapStream.cs
@@ -492,7 +492,7 @@ namespace MailKit.Net.Imap {
 			byte* inend = inbuf + inputEnd;
 			bool escaped = false;
 
-			// skip over the leading '"'
+			// skip over the opening '"'
 			inptr++;
 
 			using (var memory = new MemoryStream ()) {
@@ -512,8 +512,25 @@ namespace MailKit.Net.Imap {
 					}
 
 					if (inptr < inend) {
+						// skip over closing '"'
 						inptr++;
-						break;
+
+						// Note: Some IMAP servers do not properly escape double-quotes inside
+						// of a qstring token and so, as an attempt at working around this
+						// problem, check that the closing '"' character is not immediately
+						// followed by any character that we would expect immediately following
+						// a qstring token.
+						//
+						// See https://github.com/jstedfast/MailKit/issues/485 for details.
+						if (inptr < inend && "]) \r\n".IndexOf ((char) *inptr) != -1)
+							break;
+
+						// "fix" the unescaped quote
+						memory.WriteByte ((byte) '\\');
+						memory.WriteByte ((byte) '"');
+
+						if (inptr < inend)
+							continue;
 					}
 
 					inputIndex = (int) (inptr - inbuf);
@jstedfast

This comment has been minimized.

Copy link
Owner

commented Apr 5, 2017

Actually, probably leave out the line that writes the '\\' byte.

@jstedfast

This comment has been minimized.

Copy link
Owner

commented Apr 5, 2017

If this works for you, I'll commit it to master so it's included in the 1.14 release I plan to make this weekend.

jstedfast added a commit that referenced this issue Apr 5, 2017

@jstedfast

This comment has been minimized.

Copy link
Owner

commented Apr 5, 2017

Seems to work in my unit test, so committed.

@jstedfast jstedfast closed this Apr 5, 2017

@breedbekkikker

This comment has been minimized.

Copy link
Author

commented Apr 6, 2017

Great, thanks!

@breedbekkikker

This comment has been minimized.

Copy link
Author

commented Apr 12, 2017

Hi, I am currently testing with the latest code, including this fix, and I notice that folder listings that went OK before the fix are now failing with syntax error exceptions.

This may have to do with empty string subjects?

For example:

Exception 1:

Invalid INTERNALDATE format: 23-Oct-2016 19:35:59 +0200" RFC822.SIZE 2548 ENVELOPE ("Tue, 16 Jun 2015 19:35:55 +0200

Protocol log 1:

S: * 11 FETCH (UID 11 FLAGS () INTERNALDATE "23-Oct-2016 19:35:59 +0200" RFC822.SIZE 2548 ENVELOPE ("Tue, 16 Jun 2015 19:35:55 +0200" "" ((NIL NIL "vernicehintz" "acme.com")) ((NIL NIL "vernicehintz" "acme.com")) ((NIL NIL "vernicehintz" "acme.com")) (("Yadira Morissette" NIL "yadiramorissette" "acme.com")) NIL NIL NIL "<CUTVI36QKZT4.QB65OFVUV7IK3@acme.com>") BODYSTRUCTURE (("TEXT" "PLAIN" ("charset" "utf-8") NIL NIL "7BIT" 1006 2)("TEXT" "HTML" ("charset" "utf-8") "<CUTVI36QKZT4.JZOVGTW57J8G1@acme.com>" NIL "7BIT" 1018 2) "ALTERNATIVE" ("boundary" "=-uDH0rr5K8Rn1lu+CJ5z38Q==")))

Exception 2:

Syntax error in BODYSTRUCTURE. Unexpected token: [qstring: "=-QOqS83Rhl39TWl5/q+Yw7g=="]

Protocol log 2:

S: * 6 FETCH (UID 6 FLAGS () INTERNALDATE "23-Oct-2016 20:40:02 +0200" RFC822.SIZE 2198 ENVELOPE ("Mon, 25 May 2015 20:39:59 +0200" "" ((NIL NIL "alfredopurdy" "acme.com")) ((NIL NIL "alfredopurdy" "acme.com")) ((NIL NIL "alfredopurdy" "acme.com")) (("Chelsey Altenwerth" NIL "chelseyaltenwerth" "acme.com")("Hardy Lehner" NIL "hardylehner" "acme.com")("Genesis Cremin" NIL "genesiscremin" "acme.com")) NIL NIL NIL "<C7NV6RNQKZT4.F1PS0OMUPPON1@acme.com>") BODYSTRUCTURE (("TEXT" "PLAIN" ("charset" "utf-8") NIL NIL "7BIT" 746 1)("TEXT" "HTML" ("charset" "utf-8") "<C7NV6RNQKZT4.1SWILXG3K4Z71@acme.com>" NIL "7BIT" 842 1) "ALTERNATIVE" ("boundary" "=-QOqS83Rhl39TWl5/q+Yw7g==")))

jstedfast added a commit that referenced this issue Apr 12, 2017

Possible fix for the work-around for issue #485
If the new problem reported in comment-293553102 is a
boundary issue (e.g. inptr == inend after skipping the
closing " char), this should solve the problem.
@jstedfast

This comment has been minimized.

Copy link
Owner

commented Apr 12, 2017

Just committed a potential fix. If you could test that out for me and let me know, that would be great.

I think it's probably more likely to be a read boundary issue, but I can't be certain.

@jstedfast

This comment has been minimized.

Copy link
Owner

commented Apr 12, 2017

Based on my inability to reproduce the issue in the unit tests, I think it's gotta be a read boundary issue and that commit should fix things for you, but confirmation would be great to have.

If you can confirm for me, I'll push a new emergency release tonight after work.

@breedbekkikker

This comment has been minimized.

Copy link
Author

commented Apr 12, 2017

Thanks, I am testing it now...

@breedbekkikker

This comment has been minimized.

Copy link
Author

commented Apr 12, 2017

Unfortunately I have to say that it doesn't fix the issues. Do you have any other ideas?

jstedfast added a commit that referenced this issue Apr 12, 2017

@jstedfast

This comment has been minimized.

Copy link
Owner

commented Apr 12, 2017

I just committed a better fix if it is a read-boundary issue. If this doesn't fix it, I'm out of ideas.

@jstedfast

This comment has been minimized.

Copy link
Owner

commented Apr 12, 2017

Was it breaking in the same place as before?

@breedbekkikker

This comment has been minimized.

Copy link
Author

commented Apr 12, 2017

I am testing the latest fix and I notice that the application is now using 100% CPU. Is it possible that the fix can create an infinite loop?

@jstedfast

This comment has been minimized.

Copy link
Owner

commented Apr 12, 2017

I wouldn't think so in the sense that there's always something that needs to come after an end-quote, but I suppose it's possible.

@jstedfast

This comment has been minimized.

Copy link
Owner

commented Apr 12, 2017

wait, yes... ReadAhead() will shortcut if it thinks it has enough data (in this case: 1 byte)

jstedfast added a commit that referenced this issue Apr 12, 2017

@jstedfast

This comment has been minimized.

Copy link
Owner

commented Apr 12, 2017

Try that commit.

@breedbekkikker

This comment has been minimized.

Copy link
Author

commented Apr 12, 2017

That works! :) The infinite loop and the new exceptions are gone now.

@jstedfast

This comment has been minimized.

Copy link
Owner

commented Apr 12, 2017

Awesome! I'll push a release.

@breedbekkikker

This comment has been minimized.

Copy link
Author

commented Apr 12, 2017

That would be great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.