emersion / go-message

✉️ A streaming Go library for the Internet Message Format and mail messages
MIT License
373 stars 108 forks source link

Tolerate (incorrect) folding whitespace used in base64 encoded body content. #152

Closed Spiral90210 closed 2 years ago

Spiral90210 commented 2 years ago

Fixes #151

Spiral90210 commented 2 years ago

Any thoughts on accepting this PR? Fixes a problem we see with real world mail, yes it technically shouldn't be happening, but as with most things email there's always something writing mail that does so badly. Always.

emersion commented 2 years ago

Can we maybe replace ' ' and '\t' with '\n' which already gets ignored by encoding/base64? That way no need to deal with count/offset changes etc.

Spiral90210 commented 2 years ago

It's actually based on the code from base64.go, just extended to account for these extra characters - I figured extending that would make more sense than wrapping/filtering twice.

Here's the original code from the standard library:

type newlineFilteringReader struct {
    wrapped io.Reader
}

func (r *newlineFilteringReader) Read(p []byte) (int, error) {
    n, err := r.wrapped.Read(p)
    for n > 0 {
        offset := 0
        for i, b := range p[:n] {
            if b != '\r' && b != '\n' {
                if i != offset {
                    p[offset] = b
                }
                offset++
            }
        }
        if offset > 0 {
            return offset, err
        }
        // Previous buffer entirely whitespace, read again
        n, err = r.wrapped.Read(p)
    }
    return n, err
}
Spiral90210 commented 2 years ago

Can we maybe replace ' ' and '\t' with '\n' which already gets ignored by encoding/base64? That way no need to deal with count/offset changes etc.

Done - yes it is a lot more simple, thanks for the suggestion!

emersion commented 2 years ago

I figured extending that would make more sense than wrapping/filtering twice.

Hm but the wrapping is done internally by the standard library anyways right?

emersion commented 2 years ago

Thanks!

Spiral90210 commented 2 years ago

I figured extending that would make more sense than wrapping/filtering twice.

Hm but the wrapping is done internally by the standard library anyways right?

Yup 🙄 clearly I was just satisfied with it working and then just went to live in fantasy land for a while!