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

Smtp responses fixed and random wait added #63

Merged
merged 3 commits into from
May 16, 2017

Conversation

HashCode55
Copy link
Contributor

@HashCode55 HashCode55 commented Mar 31, 2017

This PR is a step towards fixing #53 and #25.
Better SMPT mimicking and adds random response time (only for port 25, not global)

@glaslos glaslos requested a review from furusiyya April 4, 2017 09:20
smtp.go Outdated
client.w("250 OK")
} else if strings.Compare(query, "DATA") == 0 {
client.w("354 End data with <CRLF>.<CRLF>")
for strings.Compare(client.r(g), ".\r\n") != 0 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

May be you want to add limit over reading data. Like break the loop when condition len(data) > dataVerbMaxLength comes true.
I think dataVerbMaxLength should be a constant.

Second Comment: You should log data you are reading

Copy link
Collaborator

@furusiyya furusiyya left a comment

Choose a reason for hiding this comment

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

Over all it looks good, I made some inline comments where changes are required.

smtp.go Outdated
@@ -9,6 +9,9 @@ import (
"time"
)

// maximum lines that can be read after the "DATA" command
var maxDataRead int = 500
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not constant?

Copy link
Contributor Author

@HashCode55 HashCode55 Apr 4, 2017

Choose a reason for hiding this comment

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

I made a commit like 10 seconds before you commented haha

@glaslos glaslos merged commit 8ec2df8 into mushorg:master May 16, 2017
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.

3 participants