Skip to content

panic on hotmail/outlook.com IMAP fetch #11

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

Closed
chshawkn-pub opened this issue Apr 29, 2016 · 13 comments
Closed

panic on hotmail/outlook.com IMAP fetch #11

chshawkn-pub opened this issue Apr 29, 2016 · 13 comments

Comments

@chshawkn-pub
Copy link

chshawkn-pub commented Apr 29, 2016

I put content of example.rs into a test of my project, use imap-mail.outlook.com:993.

I got a panic at 'imap_socket.fetch("2", "body[text]");'.

My env is rust-1.8.0

thread 'test_imap_fetch' panicked at 'calledResult::unwrap()on anErrvalue: FromUtf8Error { bytes: [215, 238, 189, 252, 180, 211, 32, 77, 105, 99, 114, 111, 115, 111, 102, 116, 32, 213, 202, 187, 167, 32, 118, 101, 42, 42, 42, 42, 42, 64, 111, 117, 116, 108, 111, 111, 107, 46, 99, 111, 109, 32, 214, 208, 201, 190, 179, 253, 193, 203, 210, 212, 207, 194, 176, 178, 200, 171, 208, 197, 207, 162, 58, 13, 10], error: Utf8Error { valid_up_to: 0 } }', ../src/libcore/result.rs:746 stack backtrace: 1: 0x10a5dfae8 - sys::backtrace::tracing::imp::write::h3b28049ffa6d6406ZZu 2: 0x10a5e1875 - panicking::default_handler::_$u7b$$u7b$closure$u7d$$u7d$::closure.43942 3: 0x10a5e13d7 - panicking::default_handler::h9792cd244a79d9daorz 4: 0x10a5d4366 - sys_common::unwind::begin_unwind_inner::h127fbab41243a988LYt 5: 0x10a5d481e - sys_common::unwind::begin_unwind_fmt::h849e306453bce5feRXt 6: 0x10a5df007 - rust_begin_unwind 7: 0x10a6060f0 - panicking::panic_fmt::h98b8cbb286f5298alcM 8: 0x10a4f4b29 - result::unwrap_failed::h7834371121640111038 9: 0x10a4f4945 - result::Result<T, E>::unwrap::h5359275872070956371 10: 0x10a4f0837 - client::IMAPStream::read_response::haf4a98f70dfa51f1hEa 11: 0x10a4de1c2 - client::IMAPStream::run_command::h920cc295b4f173d4kza 12: 0x10a4ed351 - client::IMAPStream::fetch::h2876f40048060ec4Upa 13: 0x10a4bb36e - mail::imap_fetch::h3d57dadf089b9b98Maa 14: 0x10a497848 - test_imap_fetch::h8dacc3008846b6edgaa 15: 0x10a4b578b - boxed::F.FnBox<A>::call_box::h9454666050355180836 16: 0x10a4b7bfd - sys_common::unwind::try::try_fn::h13611093186668648166 ...

@chshawkn-pub
Copy link
Author

Should not assume encoding is UTF-8 at client.rs:301 let line = String::from_utf8(line_buffer).unwrap();

@mattnenterprise
Copy link
Owner

@zhl-home1 How do you determine the encoding?

@chshawkn-pub
Copy link
Author

That is a problem. There is a lib 'rust-uchardet'. I've used other library in Java, which can give probability result. Is it possible that provide an additional argument let user specify encoding and produce an encoding error instead of a panic.

@mattnenterprise
Copy link
Owner

I'm still not sure of the proper way to fix this. I'm not really sure how to test out any solution I could come up with. @chshawkn-pub Could you help with this ?

@vandenoever
Copy link
Contributor

I created a PR but it's not the right solution. #32

fetch and uid_fetch should determine the number of bytes to read from the first line of the response. It's the value between {}. Read that as Vec<u8> and return it as is.

To make sense of the mail, use the mailparse crate. Parsing mail is not trivial.

@vandenoever
Copy link
Contributor

#33 introduces a new API. fetch and uid_fetch now return bytes instead of lines.

@dario23
Copy link

dario23 commented May 13, 2017

is there anything blocking this? i stumbled over it myself, and would really look forward to this being fixed and #33 being merged 👍

@jonhoo
Copy link
Collaborator

jonhoo commented Oct 1, 2017

@dario23 + others: I just closed #33, since it is not really the right solution, and presents a much less ergonomic API. See the reasoning in my comment there (#33 (comment)). If this is still a major issue, I'd suggest implementing the (apparently fairly straightforward) extensions from RFC5738, which makes the server talk UTF-8.

@sanmai-NL
Copy link
Contributor

@jonhoo: outlook.com does not have the capability for that extension:

{"msg":"Capability: IMAP4","v":0,"name":"slog-rs","level":10,"time":"2017-10-19T14:57:12.076151578+02:00"}
{"msg":"Capability: IMAP4rev1","v":0,"name":"slog-rs","level":10,"time":"2017-10-19T14:57:12.076216547+02:00"}
{"msg":"Capability: AUTH=PLAIN","v":0,"name":"slog-rs","level":10,"time":"2017-10-19T14:57:12.076258435+02:00"}
{"msg":"Capability: AUTH=XOAUTH2","v":0,"name":"slog-rs","level":10,"time":"2017-10-19T14:57:12.076299083+02:00"}
{"msg":"Capability: SASL-IR","v":0,"name":"slog-rs","level":10,"time":"2017-10-19T14:57:12.076338888+02:00"}
{"msg":"Capability: UIDPLUS","v":0,"name":"slog-rs","level":10,"time":"2017-10-19T14:57:12.076378373+02:00"}
{"msg":"Capability: MOVE","v":0,"name":"slog-rs","level":10,"time":"2017-10-19T14:57:12.076418006+02:00"}
{"msg":"Capability: ID","v":0,"name":"slog-rs","level":10,"time":"2017-10-19T14:57:12.076457521+02:00"}
{"msg":"Capability: UNSELECT","v":0,"name":"slog-rs","level":10,"time":"2017-10-19T14:57:12.076497288+02:00"}
{"msg":"Capability: CLIENTACCESSRULES","v":0,"name":"slog-rs","level":10,"time":"2017-10-19T14:57:12.076537634+02:00"}
{"msg":"Capability: CLIENTNETWORKPRESENCELOCATION","v":0,"name":"slog-rs","level":10,"time":"2017-10-19T14:57:12.076578680+02:00"}
{"msg":"Capability: BACKENDAUTHENTICATE","v":0,"name":"slog-rs","level":10,"time":"2017-10-19T14:57:12.076619247+02:00"}
{"msg":"Capability: CHILDREN","v":0,"name":"slog-rs","level":10,"time":"2017-10-19T14:57:12.076658928+02:00"}
{"msg":"Capability: IDLE","v":0,"name":"slog-rs","level":10,"time":"2017-10-19T14:57:12.076698351+02:00"}
{"msg":"Capability: NAMESPACE","v":0,"name":"slog-rs","level":10,"time":"2017-10-19T14:57:12.076738295+02:00"}
{"msg":"Capability: LITERAL+","v":0,"name":"slog-rs","level":10,"time":"2017-10-19T14:57:12.076778015+02:00"}

It appears to run Microsoft Exchange in some variant. This kind of server has a large market share. We should really do better to support this.

@dario23
Copy link

dario23 commented Oct 19, 2017

@jonhoo the rfc also says

The IMAP server MUST NOT perform up-conversion of headers and content of multipart/signed, as well as Original-Recipient and Return-Path.

so clients will have to be able to deal with non-utf8 message content in any case (and i'm still not sure if the UTF8 support in that RFC does imply message bodies at all, it only explicitly mentions various imap commands that support utf8 parameters if the server advertises the relevant capabilities).

@sanmai-NL
Copy link
Contributor

sanmai-NL commented Oct 19, 2017

@jonhoo: In general, not all code depending on this library needs or has resources to decode all IMAP data as UTF-8. An API such as in #33 should be available at least. We can make the API convenient by making the current functions generic by wrapping them in a trait with an associated type String or Vec<u8>.

@sanmai-NL
Copy link
Contributor

#52 is a first step.

@jonhoo
Copy link
Collaborator

jonhoo commented Oct 19, 2017

I merged #52, which technically closes this issue. Discussion continued in #54.

@jonhoo jonhoo closed this as completed Oct 19, 2017
mtorromeo pushed a commit to mtorromeo/rust-imap that referenced this issue Nov 28, 2024
Put 1.70 in there (for instance if you want to pin against OnceLock stabilizing) and it will actually test 1.7 as it appears github auto converts this to a float?

Putting in quotes seems to do the right thing here
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants