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

Bugfix/SMTP client nil dereference #82

Merged

Conversation

RichiMaulana
Copy link
Contributor

  • Added error handler when opening new SMTP Client

PR Details

Fix bug when validate into MX server that instantly reject the connection.

Description

Before, error in the SMTP client creation in the smtp_client.go / smtpClient.runSession are not handled like below:

client, _ := smtp.NewClient(connection, smtpClient.targetServerAddress)

That behavior resulting runtime error in program execution, specifically nil pointer dereference. Below are the errors when it happen:

panic: runtime error: invalid memory address or nil pointer dereference
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x0 pc=0x100a2bda8]

goroutine 1 [running]:
net/smtp.(*Client).Close(0x140000021a0?)
	/usr/local/go/src/net/smtp/smtp.go:78 +0x18
panic({0x100b618e0?, 0x100d7de20?})
	/usr/local/go/src/runtime/panic.go:920 +0x26c
net/smtp.(*Client).Hello(0x0, {0x100ab6293, 0xe})
	/usr/local/go/src/net/smtp/smtp.go:102 +0x38
github.com/truemail-rb/truemail-go.(*smtpClient).runSession(0x1400042f580)
	/Users/richisetyamaulana/go/pkg/mod/github.com/truemail-rb/truemail-go@v1.1.2/smtp_client.go:110 +0x1fc
github.com/truemail-rb/truemail-go.(*validationSmtp).runSmtpSession(0x14000113c20, {0x1400042a0f0?, 0x40?})
	/Users/richisetyamaulana/go/pkg/mod/github.com/truemail-rb/truemail-go@v1.1.2/smtp.go:65 +0x170
github.com/truemail-rb/truemail-go.(*validationSmtp).run(0x14000113c20)
	/Users/richisetyamaulana/go/pkg/mod/github.com/truemail-rb/truemail-go@v1.1.2/smtp.go:42 +0x88
github.com/truemail-rb/truemail-go.(*validationSmtp).check(0x14000113c20, 0x140001326e0)
	/Users/richisetyamaulana/go/pkg/mod/github.com/truemail-rb/truemail-go@v1.1.2/smtp.go:14 +0x7c
github.com/truemail-rb/truemail-go.(*validator).validateSMTP(0x140003dfcb0)
	/Users/richisetyamaulana/go/pkg/mod/github.com/truemail-rb/truemail-go@v1.1.2/validator.go:145 +0x318
github.com/truemail-rb/truemail-go.(*validator).run(0x140003dfcb0)
	/Users/richisetyamaulana/go/pkg/mod/github.com/truemail-rb/truemail-go@v1.1.2/validator.go:171 +0xd8
github.com/truemail-rb/truemail-go.Validate({0x100ac6c81, 0x17}, 0x140001910a0, {0x0?, 0x100ab6293?, 0xe?})
	/Users/richisetyamaulana/go/pkg/mod/github.com/truemail-rb/truemail-go@v1.1.2/truemail.go:13 +0x22c
main.validate({0x100ac6c81, 0x17})
	/Users/richisetyamaulana/Excellent/dev/aktiva-email-validator/main.go:148 +0x74
main.main()
	/Users/richisetyamaulana/Excellent/dev/aktiva-email-validator/main.go:80 +0x6c
exit status 2

Related Issue

Motivation and Context

  • To fix the runtime error when validating into rejected MX server

How Has This Been Tested

I already try to reproduce the error by validating into MX server that instantly reject the SMTP connection using my bug fix version. The results are the error are not appearing again.

Example of MX server that instantly reject can be test using telnet like below:

$ telnet mx02.mail.icloud.com. 25
Trying 17.42.251.62...
Connected to mx02.mail.icloud.com.
Escape character is '^]'.
Connection closed by foreign host.

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project
  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have read the CONTRIBUTING document
  • I have added tests to cover my changes

* Added error handler when opening new SMTP Client
@bestwebua
Copy link
Member

Hi, @RichiMaulana! Thanks for your involvement. It's the best way to fix this issue. I guess in case when something wrong with connection this line of code will run and raise first:

connection, err := smtpClient.initConnection()

if err != nil {
  smtpClient.err = &SmtpClientError{isConnection: true, err: err}
  return false
}

But no, as we see we should expect possible error here:

client, _ := smtp.NewClient(connection, smtpClient.targetServerAddress)

So we need somehow to cover this update in tests also. Seems that I need to implement new smtp mock feature to cover this behavior (dropping connection before helo command).

Copy link
Member

@bestwebua bestwebua left a comment

Choose a reason for hiding this comment

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

Great, thank you! @RichiMaulana

@bestwebua bestwebua merged commit fb2eeca into truemail-rb:develop Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

[BUG] There is an issue in the Validate function where it panics, root cause is in the runSession function
2 participants