Hi

Doesn't it make sense that org.springframework.boot.boot.actuate.endpoint.Sanitizer can be put as a generic spring utility not linked to spring-boot-actuator?

Comment From: mhalbritter

Hello!

What's your use case? Why is the location in the actuator a problem?

Comment From: jorgerod

Hi @mhalbritter

Here is my use case.

I have a utils-module in which one utility is a deep-clones Maps that relies on Sanitizer. That utility module is totally independent from actuator but I need to add the spring-boot-actuator dependency. That is, I think the Sanitizer class would be useful for actuator-independent projects (e.g. batch applications).

For me it's not a problem but I think it would be better in spring-core for example.

Thank you

Comment From: mhalbritter

Thanks for the explanation. I'll flag it as team attention to see what the rest of the team thinks.

Comment From: bclozel

I think that in order to promote the Sanitizer to spring-core, we would need to show that it will be used in other Spring Framework modules. I don't think this is the case. We could consider moving it to spring-boot, but again, I don't think we're going to use it outside of actuator. On top of that, the mere concept of sanitizer here doesn't apply to all situations, but rather an actuator endpoint.

I don't think we should promote this concept higher in the project hierarchy.

Comment From: juliojgd

@bclozel I think you could need to sanitize any string written to (let's say) a file or a network header or payload, etc. I think the "sanitize strings" feature can be used in several network and local filesystem transactions completely unrelated with Actuator. I don't know it the "promotion" should be to spring-boot or spring core but definitely is something you can need for non Actuator related features.

Comment From: bclozel

I don't think the Sanitizer was meant to serve as a general abstraction to sanitize any Sting in any situation. Would that have helped if we called it EndpointSanitizer?

Comment From: juliojgd

I don't think the Sanitizer was meant to serve as a general abstraction to sanitize any Sting in any situation. Would that have helped if we called it EndpointSanitizer?

Maybe it's helped.

But, do we agree that the "sanitization" mechanism is something it's needed in several non-Actuator pieces both in Spring Boot as well as applications using it?

Maybe other pieces of Spring/SB needed it and already implemented it independently (more than once). That's the root reason to the proposal of @jorgerod , if it's needed in several places to have that feature in a common place, more general than Actuator codebase.

And thanks for your reply, of course 😄

Comment From: philwebb

In my opinion we should keep Sanitizer focused as an actuator specific concern. Although it's tempting to try and generalize it further, I'm not convinced we're going to gain a lot by moving it to spring-core.

There are few things in the current API which make me think it's best where it is. Namely:

  • SanitizableData has a getPropertySource() method which allows a sanitizing function to sanitize all items from a specific property source. This is a somewhat leaky abstraction which feels like an OK compromise for an endpoint specific API but not really ideally if we move things to spring-core.
  • The Sanitizer.sanitize method has a showUnsanitized. This again is very endpoint specific and is really there to support properties such as management.endpoint.env.show-values=WHEN_AUTHORIZED.

We've also needed to make changes to the Sanitizer API in the past. This isn't too hard when it's an endpoint specific API, but will be much more difficult if it's a shared core API.

Comment From: mhalbritter

Thanks team! I'll close this one then.