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
Boundary reader - long lines processing fixed #200
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice find.
boundary.go
Outdated
// Read whole line, handle extra long lines in cycle | ||
var segment []byte | ||
segment, err = b.r.ReadSlice('\n') | ||
line = append(line, segment...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A concern I have here is we are going to be doing a lot more allocations than we used to. In the previous version of this loop, line
was just a pointer into the bufio buffer, but now we are allocating and growing line
multiple times per Next().
Is it feasible to move the line buffer into the b
struct and re-use it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about allocating only when the line is longer than buffer?
In fact, this is not a bug at all. MIME mail must have at most 998 octects on the line according to RFC, so it's about tolerance of some existing malformed emails.
boundary_test.go
Outdated
@@ -477,6 +477,24 @@ func TestBoundaryReaderReadErrors(t *testing.T) { | |||
} | |||
} | |||
|
|||
func TestBoundaryReaderLongLine(t *testing.T) { | |||
//dest := make([]byte, 8 *1024) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, removed. I can squash the fixes and force push as a single commit if you want. I'm not sure if github can do this automatically like gitlab.
I like the new approach, thanks. |
Hi,
when there is a line longer than 4096 bytes, message parsing fails, because boundary reader returns bufio.ErrBufferFull.
This patch handles reading longer lines than 4096 bytes.