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

feat: Implement Close. #24

Closed
wants to merge 3 commits into from
Closed

feat: Implement Close. #24

wants to merge 3 commits into from

Conversation

iredmail
Copy link

@iredmail iredmail commented Aug 22, 2022

This PR implements Close() method for the Milter interface.
fixes #20

session.go Outdated
@@ -210,8 +210,7 @@ func (m *milterSession) Process(msg *Message) (Response, error) {

case CodeQuit:
// client requested session close
return nil, errCloseSession

return m.backend.Close(newModifier(m))
Copy link
Owner

Choose a reason for hiding this comment

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

Before errCloseSession was returned, now it's never returned

Copy link
Author

@iredmail iredmail Aug 22, 2022

Choose a reason for hiding this comment

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

Sorry about this mistake.

Quote from sendmail milter doc: "Since the connection is already closing, the return value is currently ignored." So, How about not return any value in Close()?

Close(m *Modifier) 

Then we can do this in Process():

	case CodeQuit:
 		m.backend.Close(newModifier(m))
 		return nil, errCloseSession

Copy link
Author

Choose a reason for hiding this comment

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

Updated PR. Please help review. @emersion

@iredmail
Copy link
Author

Could you help review again? @emersion @foxcpp

@iredmail iredmail requested a review from emersion August 25, 2022 05:46
@iredmail
Copy link
Author

Dear @emersion, could you help review again?

1 similar comment
@iredmail
Copy link
Author

Dear @emersion, could you help review again?

@emersion
Copy link
Owner

  • Since the connection is disconnecting anyways, I don't think there is value in passing a *Modifier. The other end of the connection won't see any new messages.
  • Would be nicer to implement the standard io.Closer interface.
  • This should be called even if the other end of the connection doesn't send CodeQuit (e.g. if it dies in the middle of a milter sequence).

@iredmail iredmail closed this by deleting the head repository Sep 2, 2023
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.

server: add Milter.Close
2 participants