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

StringUtils.parseLocaleString parses invalid locales successfully [SPR-17516] #22048

Closed
spring-issuemaster opened this issue Nov 18, 2018 · 4 comments

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Nov 18, 2018

Petar Tahchiev opened SPR-17516 and commented

Prior to spring-core 5.1.x calling this method:

StringUtils.parseLocaleString("some-locale-that-does-not-exist")

would throw IllegalArgumentException. However in spring-core 5.1.x (tested with 5.1.3.BUILD-SNAPSHOT) it doesn't. It simply returns a java.util.Locale object with language="some-locale-that-does-not-exist", region="", country=""


Affects: 5.1.2

Issue Links:

  • #21241 CookieLocaleResolver is not RFC6265 compliant when setting a locale and time zone
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Nov 19, 2018

Juergen Hoeller commented

As of 5.1, we only really accept "-" as a valid character in locale strings, driven by #21241 and the parsing of RFC6265 language tags. So the reason why that particular test would fail is a side effect of that "-" leniency; we would have accepted any other non-existing locale before and simply accept "-" in such non-existent locale specifications as well now...

Do you see a specific issue with the tolerance of "-" here? Or did you expect StringUtils to reject non-existent locales to begin with?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Nov 19, 2018

Petar Tahchiev commented

My expectation was that StringUtils would reject non-existing locales. However I wasn't aware of RFC6265 and it's implications. Having said that I don't really know what the correct behaviour should be.

@DeCaMil

This comment has been minimized.

Copy link

@DeCaMil DeCaMil commented Jun 21, 2019

Given that the Locale constructor itself does not validate that the Locale exists, I wouldn't expect StringUtils should do that either.

In my mind, it's no different than creating a Url object for a host that does not exist.

@sbrannen

This comment has been minimized.

Copy link
Member

@sbrannen sbrannen commented Jun 25, 2019

Given that the Locale constructor itself does not validate that the Locale exists, I wouldn't expect StringUtils should do that either.

That's a very valid argument, and we note that the following have the same output accordingly.

System.out.println(StringUtils.parseLocaleString("some-locale-that-does-not-exist"));
System.out.println(new Locale("some-locale-that-does-not-exist"));

In my mind, it's no different than creating a Url object for a host that does not exist.

Yes, indeed. Moreover, the Javadoc for parseLocaleString() states the following.

For many parsing scenarios, this is an inverse operation of Locale's toString, in a lenient sense. This method does not aim for strict Locale design compliance; it is rather specifically tailored for typical Spring parsing needs.

In light of the above, I am closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.