djc / imap-proto

IMAP protocol parser and datastructures in Rust

imap-proto has moved

The code for imap-proto now lives in the tokio-imap repository. This repository is kept around for now to preserve the issues filed here.

imap-proto's Issues

Private fields in `Response` type: how to get at contents?

E.g.: field `0` of tuple-struct `imap::Capabilities` is private. I couldn’t find getters or so for these fields. IMO, the fields in public types should just be public too. (The crate does not have a stable API and needs to be used in practice before even considering adding a lot of ceremony like getters.)

No support for non-UTF8 parsing

It is (unfortunately) not entirely uncommon for IMAP servers to not support UTF-8 (see mattnenterprise/rust-imap#54 for more), but imap-proto currently returns &str all over the place, which necessarily implies that it only works on UTF-8 response. I think the right thing to do here (although it makes the types a bit less pleasant) is to parameterize all the types by a T: TryFrom<&[u8]>. That way, users can choose whether to parse into str, or simply keep things as &[u8] (or do some other decoding). Unfortunately, TryFrom is still nightly-only, and hasn't even landed yet for strings (rust-lang/rust#44916), so in the meantime, you'd probably just want to add a trait like this:

pub trait FromByteResponse<'a> {
    fn from_bytes(&'a [u8]) -> Self;
}

impl<'a> FromByteResponse<'a> for &'a str {
    fn from_bytes(b: &'a [u8]) -> Self {
        use std;
        std::str::from_utf8(b).unwrap()
    }
}

impl<'a> FromByteResponse<'a> for &'a [u8] {
    fn from_bytes(b: &'a [u8]) -> Self {
        b
    }
}

And then make the various types use this trait. For example, for MailboxDatum:

pub enum MailboxDatum<T: for<'a> FromByteResponse<'a>> {
    Exists(u32),
    Flags(Vec<T>),
    List(Vec<T>, T, T),
    Recent(u32),
}

Add re-export of nom's `IResult`

Given that IResult is returned by parse_response, it would be useful for imap-proto to re-export nom::IResult (as well as perhaps the various types included in its variants) so that users of this crate do not need to explicitly pull in nom.

CommandBuilder quotation

There is a quotation bug in CommandBuilder -- we aren't applying IMAP4 quotation to passwords which is a mistake because passwords can contain " and \ (which result in parsing errors on IMAP4 servers). The solution would be to just apply the same quotation that Python's imaplib does (I've also implemented it in my own tool).

LSUB response type not supported

RFC 3501 defines the LSUB command (§6.3.9), and the associated LSUB response (§7.2.3, which are of identical form to LIST responses (except with s/LIST/LSUB/):

* LSUB () "." #news.comp.mail.mime

The parser currently chokes on this, as it only allows a LIST literal.

Fix integer overflow during parsing

cargo +nightly-2017-10-05 fuzz run utf8_parse_response -- -only_ascii=1

      Fresh arbitrary v0.1.0
       Fresh cc v1.0.3                                                                                                                                                    
       Fresh libc v0.2.33                                                                                                                                                 
       Fresh memchr v1.0.2                                                                                                                                                
       Fresh nom v3.2.1                                                                                                                                                   
       Fresh libfuzzer-sys v0.1.0 (https://github.com/rust-fuzz/libfuzzer-sys.git#737524f7)                                                                               
       Fresh imap-proto v0.1.0 (file:///home/sanmai/devel/github.com/djc/imap-proto)                                                                                      
   Compiling imap-proto-fuzz v0.0.1 (file:///home/sanmai/devel/github.com/djc/imap-proto/fuzz)                                                                            
     Running `rustc --crate-name utf8_parse_response fuzz/fuzz_targets/utf8_parse_response.rs --crate-type bin --emit=dep-info,link -C debuginfo=2 -C metadata=a03a0cad977a1d2a -C extra-filename=-a03a0cad977a1d2a --out-dir /home/sanmai/devel/github.com/djc/imap-proto/fuzz/target/x86_64-unknown-linux-gnu/debug/deps --target x86_64-unknown-linux-gnu -L dependency=/home/sanmai/devel/github.com/djc/imap-proto/fuzz/target/x86_64-unknown-linux-gnu/debug/deps -L dependency=/home/sanmai/devel/github.com/djc/imap-proto/fuzz/target/debug/deps --extern imap_proto=/home/sanmai/devel/github.com/djc/imap-proto/fuzz/target/x86_64-unknown-linux-gnu/debug/deps/libimap_proto-1f870aaee1b4c109.rlib --extern libfuzzer_sys=/home/sanmai/devel/github.com/djc/imap-proto/fuzz/target/x86_64-unknown-linux-gnu/debug/deps/liblibfuzzer_sys-76eea54a6581bf0a.rlib --cfg fuzzing -Cpasses=sancov -Cllvm-args=-sanitizer-coverage-level=3 -Zsanitizer=address -Cpanic=abort -L native=/home/sanmai/devel/github.com/djc/imap-proto/fuzz/target/x86_64-unknown-linux-gnu/debug/build/libfuzzer-sys-dfeac7893557483c/out`                                                                                                         
    Finished dev [unoptimized + debuginfo] target(s) in 0.82 secs                                                                                                         
       Fresh cc v1.0.3                                                                                                                                                    
       Fresh libc v0.2.33                                                                                                                                                 
       Fresh arbitrary v0.1.0                                                                                                                                             
       Fresh memchr v1.0.2                                                                                                                                                
       Fresh nom v3.2.1
       Fresh libfuzzer-sys v0.1.0 (https://github.com/rust-fuzz/libfuzzer-sys.git#737524f7)
       Fresh imap-proto v0.1.0 (file:///home/sanmai/devel/github.com/djc/imap-proto)
       Fresh imap-proto-fuzz v0.0.1 (file:///home/sanmai/devel/github.com/djc/imap-proto/fuzz)
    Finished dev [unoptimized + debuginfo] target(s) in 0.0 secs
     Running `fuzz/target/x86_64-unknown-linux-gnu/debug/utf8_parse_response -artifact_prefix=/home/sanmai/devel/github.com/djc/imap-proto/fuzz/artifacts/utf8_parse_response/ -only_ascii=1 /home/sanmai/devel/github.com/djc/imap-proto/fuzz/corpus/utf8_parse_response`
INFO: Seed: 408015737
INFO: Loaded 0 modules (0 guards): 
Loading corpus dir: /home/sanmai/devel/github.com/djc/imap-proto/fuzz/corpus/utf8_parse_response
INFO: -max_len is not provided, using 64
#0      READ units: 89
#89     INITED cov: 2409 corp: 69/1418b exec/s: 0 rss: 155Mb
thread '<unnamed>' panicked at 'called `Result::unwrap()` on an `Err` value: ParseIntError { kind: Overflow }', /checkout/src/libcore/result.rs:906:4
note: Run with `RUST_BACKTRACE=1` for a backtrace.
==15504== ERROR: libFuzzer: deadly signal
    #0 0x560481414373 in __sanitizer_print_stack_trace /checkout/src/libcompiler_builtins/compiler-rt/lib/asan/asan_stack.cc:38
    #1 0x5604811519cd in fuzzer::Fuzzer::CrashCallback() /home/sanmai/.cargo/bin/git/checkouts/libfuzzer-sys-e07fde05820d7bc6/737524f/llvm/lib/Fuzzer/FuzzerLoop.cpp:280
    #2 0x560481151917 in fuzzer::Fuzzer::StaticCrashSignalCallback() /home/sanmai/.cargo/bin/git/checkouts/libfuzzer-sys-e07fde05820d7bc6/737524f/llvm/lib/Fuzzer/FuzzerLoop.cpp:264
    #3 0x56048117b487 in fuzzer::CrashHandler(int, siginfo_t*, void*) /home/sanmai/.cargo/bin/git/checkouts/libfuzzer-sys-e07fde05820d7bc6/737524f/llvm/lib/Fuzzer/FuzzerUtilPosix.cpp:37
    #4 0x7f5dd1393d9f  (/usr/lib/libpthread.so.0+0x11d9f)
    #5 0x7f5dd0de789f in __GI_raise (/usr/lib/libc.so.6+0x3489f)
    #6 0x7f5dd0de8f08 in __GI_abort (/usr/lib/libc.so.6+0x35f08)
    #7 0x5604813459f8 in panic_abort::__rust_start_panic::abort /checkout/src/libpanic_abort/lib.rs:59
    #8 0x5604813459f8 in __rust_start_panic /checkout/src/libpanic_abort/lib.rs:54

NOTE: libFuzzer has rudimentary signal handlers.
      Combine libFuzzer with AddressSanitizer or similar for better crash reports.
SUMMARY: libFuzzer: deadly signal
MS: 1 InsertRepeatedBytes-; base unit: f74b576b1b65321c5909433dc2e3f79220af8725
0x2a,0x20,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x43,
* 2222222222222222222222222222222222222222222C
artifact_prefix='/home/sanmai/devel/github.com/djc/imap-proto/fuzz/artifacts/utf8_parse_response/'; Test unit written to /home/sanmai/devel/github.com/djc/imap-proto/fuzz/artifacts/utf8_parse_response/crash-c91935afc8db1ba7c29aa83a549bc90ffac1d312
Base64: KiAyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyQw==
==15504==LeakSanitizer has encountered a fatal error.
==15504==HINT: For debugging, try setting environment variable LSAN_OPTIONS=verbosity=1:log_threads=1
==15504==HINT: LeakSanitizer does not work under ptrace (strace, gdb, etc)
MS: 1 InsertRepeatedBytes-; base unit: f74b576b1b65321c5909433dc2e3f79220af8725
0x2a,0x20,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x43,
* 2222222222222222222222222222222222222222222C
artifact_prefix='/home/sanmai/devel/github.com/djc/imap-proto/fuzz/artifacts/utf8_parse_response/'; Test unit written to /home/sanmai/devel/github.com/djc/imap-proto/fuzz/artifacts/utf8_parse_response/crash-c91935afc8db1ba7c29aa83a549bc90ffac1d312
Base64: KiAyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyQw==

test unit

* 2222222222222222222222222222222222222222222C

Prefer variant structs over variant tuples

In a couple of places, enum variant tuples are used (e.g. MailboxDatum::List). It might be better if these were variant structs instead, so that their contents become self-documenting. For example, compare:

pub enum MailboxDatum<'a> {
    // ...
    List(Vec<&'a str>, &'a str, &'a str),
    // ...
}

with

pub enum MailboxDatum<'a> {
    // ...
    List {
        flags: Vec<&'a str>,
        delimiter: &'a str,
        name: &'a str
    },
    // ...
}

Error handling improvement

@djc, #12 (comment) :

As for the unwrap() calls, I think the right approach there is to use the map_res!() macro from nom to convert those to parser errors, instead of unwrapping them in place. Could probably even do a nice macro that takes care of the parse-error-converting std::str::from_utf8() in a concise way. If either of you wants to take that one, that might be a nicely manageable project.

I will submit a PR for this.

Parse `FETCH ... BODYSTRUCTURE` response

See RFC 3501, 6.4.5, BODYSTRUCTURE. Also, MIME-IMB/MIME document series in RFC 2045.

For a motivation, see RFC 2683 3.2.1.4.

It would be great if some mentoring is provided over this (or ideally documentation).

Derive equality for simple types

Simple types such as Status and ResponseCode should derive Eq and PartialEq so that code using the library can do things like:

if status == Status::Ok {
}

rather than forcing

if let Status::Ok = status {
}

UNSEEN response data not parsed

As documented in RFC3501 §6.3.1, another valid response to SELECT and EXAMINE is

OK [UNSEEN <n>]

However, this ResponseCode does not yet seem to be supported in resp_text_code.