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

LocalValidatorFactoryBean#setProviderClass(Class) is unsafe [SPR-10640] #15268

Closed
spring-projects-issues opened this issue Jun 8, 2013 · 3 comments
Assignees
Labels
in: core status: declined

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Jun 8, 2013

Nick Williams opened SPR-10640 and commented

Currently, the signature for LocalValidatorFactoryBean's setProviderClass method is as follows:

public void setProviderClass(Class providerClass)

This is unsafe, as it would be easy for someone to inadvertently set the incorrect class as the argument to this method. Ultimately the argument passed to this method call is then passed to Validation#byProvider(Class), so the signature should really be this:

public <T extends Configuration<T>, U extends ValidationProvider<T>> void setProviderClass(Class<U> providerClass)

This will make it impossible for users of Spring's @Configuration to compile a configuration class that sets this value to an incorrect class.


Affects: 4.0 M1

Issue Links:

  • #13613 spring-context missing optional Import-Package directive for javax.validation.spi
@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jun 10, 2013

Nick Williams commented

Note: I just discovered that the fix I suggest actually used to be the case (for the most part; my suggestion is slightly more safe). In Spring 3.0 it was correct, but in Spring 3.1 it was made unsafe.

A change was made in changeset 9a61f36d3d281cdb255e2e22c95dee9bf2f58386 and then merged in changeset ee36c80ca961a5b2af233cd26a5483d57939c0af to make this unsafe. This seems like a backwards move to me. The change was "removed optional javax.validation.spi dependency (#13613)" (an OSGi thing). However, this makes no sense to me. The class javax.validation.spi.ValidationProvider is in the same artifact as javax.validation.ValidatorFactory, which LocalValidatorFactoryBean implements directly, so there's simply no advantage to removing the dependency on ValidationProvider.

So, I stand by my original suggestion and say that the change made in Spring 3.1 needs to be undone.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 26, 2014

Juergen Hoeller commented

Since this is just about guidance for which classes to pass in, I'm inclined not to change this back. The spi subpackage isn't necessarily exposed in an OSGi environment with its whitelisting of published packages, and even if the OSGi part isn't as much of a problem anymore these days, it's hard to see why somebody would get this rather specific configuration property wrong.

Juergen

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Dec 2, 2014

Nick Williams commented

I don't understand any of that (admittedly, I don't do OSGi). How does that relate to this being a compiler warning, and the fact that it USED to be safe and someone (IMO, inappropriately) changed it from safe to unsafe?

@spring-projects-issues spring-projects-issues added type: bug status: declined in: core labels Jan 11, 2019
@spring-projects-issues spring-projects-issues removed the type: bug label Jan 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core status: declined
Projects
None yet
Development

No branches or pull requests

2 participants