Store strings as &[u8] instead of &str

[link2xt]

An attempt to fix #62

[djc]

Yes, this is straightforward, however it is a setback in usability. @jonhoo, any thoughts?

[link2xt]

Currently it causes Delta Chat to stop receiving messages due to spam messages with invalid-UTF8 subjects. Users can convert using from_utf8_lossy if needed, I don't think it is a major usability problem. &[u8] seems to be the only way to keep parser zero-copy.

[jonhoo]

Hmm, I'm curious how often invalid UTF-8 encodings actually come up? I'm not really opposed to the change, it's more that I wish it wasn't necessary. rust-imap can always do the decoding on its side I suppose.

[djc]

Yeah, exactly. This is the first time it's come up on this issue tracker, which I think at least means it's reasonably rare.

[link2xt]

Hmm, I'm curious how often invalid UTF-8 encodings actually come up?

Even if normally it never happens, someone can do it on purpose to make an IMAP client unable to parse FETCH responses and stop operating. It's a security issue actually.

rust-imap can always do the decoding on its side I suppose.

Do you propose to parse FETCH responses manually, without imap_proto?

[jonhoo]

I think the real resolution is to provide both methods for operating directly on &[u8] and methods that expose &str. The former should be used for those who want low-level control over how stuff works (like Delta Chat if they worry about malicious messages), and the latter for users who just want convenient APIs (e.g., my system tray e-mail notifier).

Do you propose to parse FETCH responses manually, without imap_proto?

Ah, no, I was proposing that rust-imap would parse the &[u8] that imap-proto would output with this PR merged.

[link2xt]

@jonhoo

I think the real resolution is to provide both methods for operating directly on &[u8] and methods that expose &str.

It is possible to add convenience methods to Envelope and Address, but they will be just thin from_utf8_lossy wrappers, returning Cow<str>. Returning more convenient &str requires that a String is allocated somewhere if the string happens to contain invalid UTF-8. That is outside the scope of the parser, IMO.

The former should be used for those who want low-level control over how stuff works (like Delta Chat if they worry about malicious messages), and the latter for users who just want convenient APIs (e.g., my system tray e-mail notifier).

Looks like rust-imap exposes fetches and envelopes directly to the user now. If you want more convenient API, you can convert byte slices to Cow<str> and store it in some auxiliary structure that will be exposed to the user with a stable documented .subject() API returning &str. This way users will have a convenient API that never fails and yet does not make them deal with &[u8] or Cow<str>.

Ah, no, I was proposing that rust-imap would parse the &[u8] that imap-proto would output with this PR merged.

Cool, so we agree here.

Feel free to ping me if any changes to this PR are needed.

[jonhoo]

@link2xt Yeah, again, my proposal is that we merge this, and that rust-imap (not this crate) provide the &str convenience methods (or, possibly, Cow<str> as you suggest).

[djc]

So did you assess whether any of the other uses of nstring_utf8 should also be converted?

[djc]

Thanks!

[link2xt]

So did you assess whether any of the other uses of nstring_utf8 should also be converted?

I have converted envelope and address only, there may be more similar bugs, I did not check every nstring_utf8 in the crate.

[link2xt]

@djc
Body fields seem to be wrong too.