Changed a couple of SMTP protocol items #1
Conversation
Specifically: responds to NOOP with an OK, responds to RSET by clearing all state and message buffers, and responds to . by processing the pending message and resetting.
|
Oh by the way, this is my first time contributing on github and doing a pull request, so definitely let me know if I did something wrong! :-) |
|
Hey Greg! Before I merge your changes would you be able to write tests for the new code? Does the old test suite still pass with your code changes? Regards, |
|
Hey Aaron, The original tests still pass. I just figured out a way to test the most important change, which is the multiple-emails on a single connection before a QUIT; just committed that test. As for the NOOP and the RSET changes, I'm not sure how to test those without pushing the bytes in myself. I did test them locally with a mail agent and telnet, with @Audit=true, so they should be working fine...just not sure how to automate the testing without writing a whole bunch more code. -GR |
|
That definitely sounds reasonable, I think there's always a point of Thank again for your contribution! Would you like to change the README to Regards, Aaron Gough +1-647-746-7083 On Fri, May 6, 2011 at 4:05 PM, greinacker <
|
|
Done - thanks! |
|
Just a very small side-note: def send_multiple_mails(message = $example_mail, from_address = "smtp@test.com", to_address = "some1@test.com", count = 1)
Net::SMTP.start('127.0.0.1', 2525) do |smtp|
msg_num = 0
while (msg_num < count) do
smtp.send_message(message, from_address, to_address)
msg_num += 1
end
smtp.finish
sleep 0.01
end
endWould be be more idiomatic in Ruby written like so: def send_multiple_mails(message, from_address, to_address, count)
Net::SMTP.start('127.0.0.1', 2525) do |smtp|
count.times do
smtp.send_message(message, from_address, to_address)
end
smtp.finish
sleep 0.01
end
endLooking back at my test code I definitely should not have been using a global variable as the default for a parameter, I must have been having an off day :-p The rest of the code looks great! Good work on your first open-source contribution! |
|
There's no need to change that by the way, it's more a case of me thinking out loud! :-) |
|
Ooh, I like that a lot better; not sure why I didn't think of it. I guess you can tell my background is in C/C++/C# -style languages. :-) |
|
Haha, well there's no harm in that! You almost never see while loops used in Ruby code which is the only reason it stood out... I'm going the other way language-wise. My day job is all Ruby but most of the personal work I'm doing right now is in C, it's kind of painful going the other way as I'm sure you can imagine :-p |
|
This was already merged... |
|
Hey Aaron - I'm not all that worried about it, but I was cleaning up some of my repos, and it looks like this never actually got merged... |
|
Hey Greg! Is it ok with you if I leave it for the moment? If I merge the code now I'm not going to have the time to test it and I don't want to run the risk of breaking the gem for existing users... Thanks, |
|
Ok with me - perhaps just keep this PR open so folks know it's there if they want it. Thanks! |
|
No worries, thanks for understanding! -Aaron Aaron Gough +1-647-746-7083 On Wed, Feb 29, 2012 at 7:49 PM, Greg Reinacker <
|
|
Was this ever merged? If so, the ticket can be closed. |
|
even that this is a very old pull request you still might be interested in such a component. After a lot of improvements and work, I would like to inform you about enhanced MidiSmtpServer library. Your pull request is already addressed there. Cheers, |
Made some changes to get this working consistently with certain mail agents. Specifically:
"The end of mail data indication requires that the receiver
must now process the stored mail transaction information.
This processing consumes the information in the reverse-path
buffer, the forward-path buffer, and the mail data buffer,
and on the completion of this command these buffers are
cleared." - http://www.ietf.org/rfc/rfc0821.txt