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

SslBundle implementations do not provide useful toString() results #39137

Closed

Conversation

amparab
Copy link
Contributor

@amparab amparab commented Jan 15, 2024

Fixes #39057

Change proposed :

I have added a toString() method in the implementations for SslBundle interface.
There were two implementations :

  1. PropertiesSslBundle
  2. WebServerSslBundle

This toString() method prints the following attributes of the respective class:

  1. alias of key
  2. protocol
  3. ciphers
  4. enabledProtocols

Attaching a screenshot of the line printed when object is logged :

image

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 15, 2024
@tkrah
Copy link

tkrah commented Jan 15, 2024

What about keystore type, keystore location, truststore type and location, those would also be beneficial to know, no?

@amparab
Copy link
Contributor Author

amparab commented Jan 15, 2024

What about keystore type, keystore location, truststore type and location, those would also be beneficial to know, no?

I have added keystore type and truststore type (if truststore is not null, otherwise it would throw NullPointerException)
Printing the keystore and truststore location is little tricky, because the keystore and truststore has already been loaded at this point and it's path is not available.

Copy link
Member

@wilkinsona wilkinsona left a comment

Choose a reason for hiding this comment

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

Thanks very much for the PR, @amparab.

There's a utility in Spring Framework, ToStringCreator, that we like to use for this sort of thing. Here's one existing example in Spring Boot's code of how it's used:

@Override
public String toString() {
ToStringCreator creator = new ToStringCreator(this);
creator.append("type", this.type);
creator.append("value", (this.value != null) ? "provided" : "none");
creator.append("annotations", this.annotations);
creator.append("bindMethod", this.bindMethod);
return creator.toString();
}

Would you like to update your proposal to use it? Please don't worry if you don't have time, we can always take care of it as part of merging the change.

@tkrah
Copy link

tkrah commented Jan 16, 2024

What about keystore type, keystore location, truststore type and location, those would also be beneficial to know, no?

I have added keystore type and truststore type (if truststore is not null, otherwise it would throw NullPointerException) Printing the keystore and truststore location is little tricky, because the keystore and truststore has already been loaded at this point and it's path is not available.

Could you transport that location info (add it to the code) just for that purpose maybe?

@wilkinsona
Copy link
Member

Could you transport that location info (add it to the code) just for that purpose maybe?

That's a much broader and more complex change, particularly as some values can either be a location from which something is loaded or something inlined directly.

As a first pass, for this PR let's focus on the things that are readily available. We can then perhaps consider a second pass that does something more complex, possibly through implementation of the Origin interface.

@tkrah
Copy link

tkrah commented Jan 16, 2024

Could you transport that location info (add it to the code) just for that purpose maybe?

That's a much broader and more complex change, particularly as some values can either be a location from which something is loaded or something inlined directly.

As a first pass, for this PR let's focus on the things that are readily available. We can then perhaps consider a second pass that does something more complex, possibly through implementation of the Origin interface.

Sounds good :)

@amparab
Copy link
Contributor Author

amparab commented Jan 16, 2024

Thanks very much for the PR, @amparab.

There's a utility in Spring Framework, ToStringCreator, that we like to use for this sort of thing. Here's one existing example in Spring Boot's code of how it's used:

@Override
public String toString() {
ToStringCreator creator = new ToStringCreator(this);
creator.append("type", this.type);
creator.append("value", (this.value != null) ? "provided" : "none");
creator.append("annotations", this.annotations);
creator.append("bindMethod", this.bindMethod);
return creator.toString();
}

Would you like to update your proposal to use it? Please don't worry if you don't have time, we can always take care of it as part of merging the change.

Hello @wilkinsona. I have updated the PR :-) Please let me know if any other changes.

@wilkinsona wilkinsona changed the title Fixes #39057 : Added toString method in implementations of SslBundle SslBundle implementations do not provide useful toString() results Jan 17, 2024
@wilkinsona wilkinsona added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Jan 17, 2024
@wilkinsona wilkinsona added this to the 3.1.x milestone Jan 17, 2024
@wilkinsona wilkinsona self-assigned this Jan 17, 2024
@wilkinsona wilkinsona modified the milestones: 3.1.x, 3.1.8 Jan 17, 2024
wilkinsona pushed a commit that referenced this pull request Jan 17, 2024
@wilkinsona
Copy link
Member

@amparab thank you very much for making your first contribution to Spring Boot

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

Successfully merging this pull request may close these issues.

Provide information about the source of the trust material in SslBundle's toString()
4 participants