Skip to content
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

Sometimes doesn't parsing emails #32

Merged
merged 3 commits into from
May 16, 2024
Merged

Sometimes doesn't parsing emails #32

merged 3 commits into from
May 16, 2024

Conversation

devchanki
Copy link
Contributor

I found a problem where emails sometimes weren't parsed.

Some data events ends with .\r\n, But they doesn't finished parse whole email.
I found some rfc documents and multiline commands should finish \r\n.\r\n.

Responses to certain commands are multi-line. In these cases, which
are clearly indicated below, after sending the first line of the
response and a CRLF, any additional lines are sent, each terminated
by a CRLF pair. When all lines of the response have been sent, a
final line is sent, consisting of a termination octet (decimal code
046, ".") and a CRLF pair. If any line of the multi-line response
begins with the termination octet, the line is "byte-stuffed" by
pre-pending the termination octet to that line of the response.
Hence a multi-line response is terminated with the five octets
"CRLF.CRLF". When examining a multi-line response, the client checks
to see if the line begins with the termination octet. If so and if
octets other than CRLF follow, the first octet of the line (the
termination octet) is stripped away. If so and if CRLF immediately
follows the termination character, then the response from the POP
server is ended and the line containing ".CRLF" is not considered
part of the multi-line response.

[https://www.ietf.org/rfc/rfc1939.txt](Rfc for POP3)

Please review my code, and if there are no problems, please merge.
Opinions are always appreciated, thank you for your hard work.

multiline response finished  \r\n.\r\n
change test messages
@agsh agsh self-assigned this May 16, 2024
Repository owner deleted a comment from alaptevexternal May 16, 2024
Repository owner deleted a comment from alaptevexternal May 16, 2024
Copy link
Owner

@agsh agsh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@devchanki Hello! Thank you for your investigation and exploration the code! This is an old project and right now I don't remember how POP3 works 😄
Thanks for this PR! I left a comment for your code, maybe it would be better to do it this way, what do you think?

lib/yapople.js Outdated
@@ -158,6 +158,7 @@ function onData(data) {
if (typeof this.flow !== 'undefined') { // for first and all next data chunks
this.flow = Buffer.concat([this.flow, data]); // append chunk to buffer

if (this._command.cmd === state.RETR && this.flow.slice(-5).toString() !== '\r\n.\r\n') return;
if (
this.flow.slice(this.flow.length - 3).toString() === '.\r\n' ||
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe move the condition to the next line? Smth like this:

if (
  this.flow.slice(this.flow.length - 5).toString() === '\r\n.\r\n' || 
  sData.substr(-5) === '\r\n.\r\n'
)

Because, at line 165-166 I remove five symbols, not three at the end (I don't remember why I did this 😄)
And I think that \r\n.\r\n in the specification also should work for TOP and LIST commands

@devchanki
Copy link
Contributor Author

I completely agree with you.
At first I was afraid that my logic would affect other parts, so I wrote the code by adding if condition.

I tested my mail in retr, top, list function, and it works well.
thanks.

@devchanki
Copy link
Contributor Author

I Commited more 😄!

@agsh agsh merged commit 08f6535 into agsh:master May 16, 2024
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

Successfully merging this pull request may close these issues.

2 participants