Parser failed when list messages

[steedos]

When I send command:
SEND: W4 FETCH 3297:3316 (UID FLAGS ENVELOPE BODYSTRUCTURE)

Emailjs can not parse the result, I think it is because attachement names in BODYSTRUCTURE has \r\n

[nifgraup]

Could you set up a test case for this? Either emailjs-imap-client doesn't split/join incoming commands correctly or emailjs-imap-handler doesn't parse them. Relevant unit tests are in https://github.com/emailjs/emailjs-imap-handler/blob/master/test/imap-parser-unit.js and https://github.com/emailjs/emailjs-imap-client/blob/master/test/unit/emailjs-imap-client-imap-test.js.

[nifgraup]

@steedos is this possibly related to emailjs/emailjs-imap-handler#20 ?

[rickhall]

This is not related to emailjs/emailjs-imap-handler#20 (aka #147); however, it might be related to another issue I ran across while looking into that issue.

I have a this._incomingBuffers in _iterateIncomingBuffers() (file emailjs-imap-client-imap.js) that looks something like this [unfortunately somewhat complicated] test case:

diff --git a/test/unit/emailjs-imap-client-imap-test.js b/test/unit/emailjs-imap-client-imap-test.js
index c8822e8..22fbd83 100644
--- a/test/unit/emailjs-imap-client-imap-test.js
+++ b/test/unit/emailjs-imap-client-imap-test.js
@@ -131,7 +131,22 @@
             });
         });
 
+let s1 = '* 165957 FETCH (UID 626617 BODY[1.2] {2791}\r\n<p><b>@ibauersachs</b> commented on this pull request.</p>\r\n\r\n<hr>\r\n\r\n<p>In <a href="https://github.com/jitsi/jitsi/pull/380#discussion_r128120548">test/net/java/sip/communicator/impl/protocol/jabber/extensions/colibri/ColibriIQProviderTest.java</a>:</p>\r\n<pre style=\'color:#555\'>&gt; +import junit.framework.TestCase;\r\n+import net.java.sip.communicator.impl.protocol.jabber.SmackV3InteroperabilityLayer;\r\n+import net.java.sip.communicator.service.protocol.jabber.Abstr';
+
+let s2 = 'actSmackInteroperabilityLayer;\r\n+import org.jivesoftware.smack.packet.IQ;\r\n+import org.xmlpull.v1.XmlPullParser;\r\n+import org.xmlpull.v1.XmlPullParserException;\r\n+import org.xmlpull.v1.XmlPullParserFactory;\r\n+\r\n+import java.io.IOException;\r\n+import java.io.StringReader;\r\n+import java.util.List;\r\n+import java.util.stream.Collectors;\r\n+\r\n+public class ColibriIQProviderTest extends TestCase\r\n+{\r\n+    String testXml = &quot;\n&quot; +\r\n</pre>\r\n<p>C# ftw:</p>\r\n<pre><code>XDocument.Parse("&lt;root&gt;\r\n    &lt;asdf/&gt;    &lt;/root&gt;").ToString(SaveOptions.DisableFormatting)\r\n</code></pre>\r\n\r\n<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">&mdash;<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/jitsi/jitsi/pull/380#discussion_r128120548">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AER2YxZrq6Y21awF_VeBxVQRRiaL9bXWks5sPT2dgaJpZM4Ob4SY">mute the thread</a>.<img alt="" h';
+
+let s3 = 'eight="1" src="https://github.com/notifications/beacon/AER2Y4acu0EylRDxDNOLG9DxBF2Z8sCbks5sPT2dgaJpZM4Ob4SY.gif" width="1" /></p>\r\n<div itemscope itemtype="http://schema.org/EmailMessage">\r\n<div itemprop="action" itemscope itemtype="http://schema.org/ViewAction">\r\n  <link itemprop="url" href="https://github.com/jitsi/jitsi/pull/380#discussion_r128120548"></link>\r\n  <meta itemprop="name" content="View Pull Request"></meta>\r\n</div>\r\n<meta itemprop="description" content="View this Pull Request on GitHub"></meta>\r\n</div>\r\n\r\n<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/jitsi/jitsi","title":"jitsi/jitsi","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/jitsi/jitsi"}},"updates":{"snippets":[{"icon":"PERSON","message":"@ibauersachs commented on #380"}],"action":{"name":"View Pull Request","url":"https://github.com/jitsi/jitsi/pull/380#discussion_r128120548"}}}</script> BODY[1.1] {964}\r\nibauersachs commented on this pull request.\r\n\r\n\r\n\r\n> +import junit.framework.TestCase;\r\n+import net.java.sip.communicator.impl.protocol.jabber.SmackV3InteroperabilityLayer;\r\n+import net.java.sip.communicator.service.protocol.jabber.AbstractSmackInteroperabilityLayer;\r\n+import org.jivesoftware.smack.packet.IQ;\r\n+import org.xmlpull.v1.XmlPullParser;\r\n+import org.xmlpull.v1.XmlPullParserException;\r\n+import org.xmlpull.v1.XmlPullParserFactory;\r\n+\r\n+import java.io.IOException;\r\n+import java.io.StringReader;\r\n+import java.util.List;\r\n+import java.util.stream.Collectors;\r\n+\r\n+public class ColibriIQProviderTest extends TestCase\r\n+{\r\n+    String testXml = "\n" +\r\n\r\nC# ftw:\r\n```\r\nXDocument.Parse("<root>\r\n    <asdf/>    </root>").ToStr';
+
+
         describe('#_iterateIncomingBuffer', () => {
+            it('Parse packetized multiple literals', () => {
+                appendIncomingBuffer(s1);
+                appendIncomingBuffer(s2);
+                appendIncomingBuffer(s3);
+                var iterator = client._iterateIncomingBuffer();
+                expect(iterator.next().value).to.be.undefined;
+            });
+
             it('Parse multiple zero-length literals', () => {
                 appendIncomingBuffer('* 126015 FETCH (UID 585599 BODY[1.2] {0}\r\n BODY[1.1] {0}\r\n)\r\n');
                 var iterator = client._iterateIncomingBuffer();

In this case, there is only a partial response in this._incomingBuffers. It is my understanding that _iterateIncomingBuffers() should return an undefined result in this case. However, in this case it ends passing back the following:

* 165957 FETCH (UID 626617 BODY[1.2] {2791}\r\n<p><b>@ibauersachs</b> commented on this pull request.</p>\r\n\r\n<hr>\r\n\r\n<p>In <a href=\"https://github.com/jitsi/jitsi/pull/380#discussion_r128120548\">test/net/java/sip/communicator/impl/protocol/jabber/extensions/colibri/ColibriIQProviderTest.java</a>:</p>\r\n<pre style='color:#555'>&gt; +import junit.framework.TestCase;\r\n+import net.java.sip.communicator.impl.protocol.jabber.SmackV3InteroperabilityLayer;\r\n+importnet.java.sip.communicator.service.protocol.jabber.AbstractSmackInteroperabilityLayer;\r\n+import org.jivesoftware.smack.packet.IQ;\r\n+import org.xmlpull.v1.XmlPullParser;\r\n+import org.xmlpull.v1.XmlPullParserException;\r\n+import org.xmlpull.v1.XmlPullParserFactory;\r\n+\r\n+import java.io.IOException;\r\n+import java.io.StringReader;\r\n+import java.util.List;\r\n+import java.util.stream.Collectors;\r\n+\r\n+public class ColibriIQProviderTest extends TestCase\r\n+{\r\n+    String testXml = &quot;\n&quot; +\r\n</pre>\r\n<p>C# ftw:</p>\r\n<pre><code>XDocument.Parse(\"&lt;root&gt;\r\n    &lt;asdf/&gt;    &lt;/root&gt;\").ToString(SaveOptions.DisableFormatting)\r\n</code></pre>\r\n\r\n<p style=\"font-size:small;-webkit-text-size-adjust:none;color:#666;\">&mdash;<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href=\"https://github.com/jitsi/jitsi/pull/380#discussion_r128120548\">view it on GitHub</a>, or <a href=\"https://github.com/notifications/unsubscribe-auth/AER2YxZrq6Y21awF_VeBxVQRRiaL9bXWks5sPT2dgaJpZM4Ob4SY\">mute the thread</a>.<img alt=\"\" height=\"1\" src=\"https://github.com/notifications/beacon/AER2Y4acu0EylRDxDNOLG9DxBF2Z8sCbks5sPT2dgaJpZM4Ob4SY.gif\" width=\"1\" /></p>

Which obviously fails to parse. I'm not a sure what is going on exactly, but for whatever reason it is getting confused when it searches for line feed in this portion of _iterateIncomingBuffers():

          if (this._literalRemaining === 0 && i < buf.length) {
              const LFidx = buf.indexOf(LINE_FEED, i);
              if (LFidx > -1) {
                  if (LFidx < buf.length-1) {
                      this._incomingBuffers[this._incomingBuffers.length-1] = new Uint8Array(buf.buffer, 0, LFidx+1);
                  }
                  ...

For some reason the function treats this as a complete response and returns it. Admittedly, I don't know jack about this, so I may have the setup and/or explanation incorrect, so please enlighten me if I do.

When running in the the wild, this issue is not strictly repeatable since it depends on how the packetized response is received from the socket. So, a message that fails one time may succeed another time.

[rickhall]

Just to follow up on this, here is a simpler test case that demonstrates the current bug:

diff --git a/test/unit/emailjs-imap-client-imap-test.js b/test/unit/emailjs-imap-client-imap-test.js
index 91d18d1..d335296 100644
--- a/test/unit/emailjs-imap-client-imap-test.js
+++ b/test/unit/emailjs-imap-client-imap-test.js
@@ -132,6 +132,14 @@
         });
 
         describe('#_iterateIncomingBuffer', () => {
+            it('should ignore incomplete literals with line feeds', () => {
+                appendIncomingBuffer('* 1 FETCH (UID {1024}\r\nThis is a partial literal.');
+                appendIncomingBuffer('It should return undefined\r\nsince it is not complete.');
+                var iterator = client._iterateIncomingBuffer();
+
+                expect(iterator.next().value).to.be.undefined;
+            });
+
             it('should iterate chunked input', () => {
                 appendIncomingBuffer('* 1 FETCH (UID 1)\r\n* 2 FETCH (UID 2)\r\n* 3 FETCH (UID 3)\r\n');
                 var iterator = client._iterateIncomingBuffer();

I working on submitting a pull request to address this issue right now. Once I do, perhaps you could test it to see if it addresses the original issue. Thanks.

[nifgraup]

@rickhall I don't think that test demonstates the bug, appendIncomingBuffer should always be followed by an iteration to process the queue.