mscdex / node-imap

An IMAP client module for node.js.
MIT License
2.17k stars 382 forks source link

(Dev Question) Parser Quoted String Tests #847

Closed LoveAndCoding closed 3 years ago

LoveAndCoding commented 3 years ago

I'm currently in process of a modernization of this library (e.g. promises, streams, etc) on my own fork, and as a part of that I'm rewriting the IMAP parser. To make sure I have compatibility, I'm converting over your exists tests to jest and making sure they all pass, and I've hit a snag that I had a question about. I recognize you wrote this code 7.5 years ago so you may not know/remember the answer, but wanted to ask just in case.

In your lexer tests you have 5 tests relating to quoted strings. Tests 1, 3, and 5 all pass with my new lexer and that's all good. But in tests 2 and 4 you do something that is very confusing and I wanted to check if it was a bug in the test or a part of the spec that I'm missing.

In both you have the following string (js string format) '\\\\"IMAP\\"' which your tests and lexer (since these tests pass) break into a string token with the value (interpreted/raw value format) \\"IMAP". However, my lexer is treating the (js) "\\\\" as the value (raw) \\, which is an escaped \ and therefore does not escape the following ", splitting the string up.

In a different way

  node-imap Lexer Rewritten Lexer
input '"\\\\"IMAP\\" is terrible :\\\\"' '"\\\\"IMAP\\" is terrible :\\\\"'
output ['\\"IMAP" is terrible :\\'] ['\\', 'IMAP', '\\', 'is terrible :\\']

In my reading of the IMAP spec, I think my rewritten lexer is correct, but I also know "IMAP is terrible" and I'm wondering if there was a reason your code/test interpreted the result you got? The commit message doesn't specify, and the triple slash (as exampled by Test 5) works fine. So just wanted to see if this is maybe a test issue, or if I'm missing something in my lexer.

Once again, not sure if you'll recall, but wanted to ask just in case.

mscdex commented 3 years ago

I couldn't tell you for sure, that was way too long ago.

LoveAndCoding commented 3 years ago

Figured that might be the case but thought I'd ask anyway. Thanks for your help (and generally for this library)!