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

Health actuator mail details shows the port as -1 when using the default port #35247

Closed
OrangeDog opened this issue May 3, 2023 · 12 comments
Closed
Assignees
Labels
type: bug A general bug
Milestone

Comments

@OrangeDog
Copy link
Contributor

When mail is autoconfigured and spring.mail.port is not defined, the health contributor shows the port as -1 instead of what is actually used.

"mail": {
  "status": "UP",
  "details": {
    "location": "smtp.example.com:-1"
  }
}
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label May 3, 2023
@wilkinsona wilkinsona added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels May 3, 2023
@wilkinsona wilkinsona added this to the 2.7.x milestone May 3, 2023
@wilkinsona wilkinsona self-assigned this May 3, 2023
@wilkinsona wilkinsona modified the milestones: 2.7.x, 2.7.12 May 3, 2023
@OrangeDog
Copy link
Contributor Author

This doesn't seem to have worked. Now it shows smtp.example.com, while the changelog implies it should be e.g. smtp.example.com:25 depending on the protocol settings.

@wilkinsona wilkinsona changed the title Health actuator mail details does not show default port Health actuator mail details shows the port as -1 when using the default port May 18, 2023
@wilkinsona
Copy link
Member

Now it shows smtp.example.com

That's intentional. The port isn't shown when using the default, just as it typically isn't shown on web addresses that are using port 80.

the changelog implies it should be e.g. smtp.example.com:25

I've retitled the issue and updated the changelog to match. Hopefully that removes the confusion.

@OrangeDog
Copy link
Contributor Author

OrangeDog commented May 18, 2023

Ah ok. I was hoping it would confirm which port it's ended up using. We can tell for a web address because it says http: or https:, but SMTP it depends on the other settings (which can also be defaults).

@wilkinsona
Copy link
Member

Unfortunately, that information isn't readily available. We'd need to be able to call org.springframework.mail.javamail.JavaMailSenderImpl.getTransport(Session) and then getURLName().getPort() but getTransport is protected so we cannot do so. The best we can do is get the port with which the mail sender has been configured. That's either a positive value or -1 to indicate that the default port for the protocol should be used.

@OrangeDog
Copy link
Contributor Author

OrangeDog commented May 18, 2023

A ticket with spring-core to make it public then perhaps? Or a cheeky subclass.

@wilkinsona
Copy link
Member

A sub-class won't be reliable as someone may define their own bean. If this is something that'd like to see, I think a Framework issue is the next step you should take.

@sbrannen
Copy link
Member

We'd need to be able to call org.springframework.mail.javamail.JavaMailSenderImpl.getTransport(Session) and then getURLName().getPort()

Based on preliminary inspection, it appears that transport.getURLName().getPort() might always return -1 for the default port scenario.

At least, based on a quick glance at the code (com.sun.mail.smtp.SMTPTransport.protocolConnect(String, int, String, String) and com.sun.mail.smtp.SMTPTransport.openServer(String, int)) it doesn't appear that the "actual port used" is available after the connection has been opened.

@wilkinsona, were you able to verify that getURLName().getPort() returns the actual port with a running mail service?

@sbrannen
Copy link
Member

As a side note, what happens with regard to what MailHealthIndicator reports as the port if the user has configured JavaMailSenderImpl with a Java Mail Properties file containing an entry for mail.smtp.port/mail.smtps.port which differs from the default?

@wilkinsona
Copy link
Member

I thought I had, but it looks like I was mistaken. Looking at the code again now and stepping through things in the debugger, I agree with your assessment, Sam. As far as I can tell, the port is updated locally for logging purposes but isn't updated in any state that can be subsequently accessed. This leaves the URLName with a port of -1. Sorry for the wild goose chase here.

@sbrannen
Copy link
Member

As far as I can tell, the port is updated locally for logging purposes but isn't updated in any state that can be subsequently accessed.

That was the same conclusion I came to.

Sorry for the wild goose chase here.

No worries. Thanks for confirming there's nothing we can do here. I'll close spring-projects/spring-framework#30507 accordingly.

Though I'm still curious about #35247 (comment).

@wilkinsona
Copy link
Member

what happens with regard to what MailHealthIndicator reports as the port if the user has configured JavaMailSenderImpl with a Java Mail Properties file containing an entry for mail.smtp.port/mail.smtps.port which differs from the default?

The health indicator uses the values returned from getHost() and getPort() on JavaMailSenderImpl. As those accessors don't take into account property-based configured, the health indicator will show the wrong values. Thankfully, that's unlikely to occur in a Boot app where, from what I've seen, Properties are rarely used to configure the sender.

@sbrannen
Copy link
Member

Thankfully, that's unlikely to occur in a Boot app where, from what I've seen, Properties are rarely used to configure the sender.

Fair enough. 👍

Thanks for elaborating.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants