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

Don't throw an exception in CookieLocaleResolver, fallback to default locale instead #22861

Closed
darioseidl opened this issue Apr 30, 2019 · 4 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone

Comments

@darioseidl
Copy link

Affects: Spring Framework 5.1.2

In the CookieLocaleResolver, would it be possible to ignore the parsing error altogether instead of throwing an IllegalStateException? Basically, I would want the CookieLocaleResolver to return the locale from the cookie, if valid, or the default locale otherwise - never throwing an exception.

It's easy enough to write a custom CookieLocaleResolver, but this seems like a bug to me. When a user has an invalid locale cookie it should not break anything, just fallback to the default locale.

This is similar to #19748 where the exception was swallowed for when it's an error dispatch.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Apr 30, 2019
@jhoeller jhoeller self-assigned this Apr 30, 2019
@jhoeller jhoeller added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Apr 30, 2019
@jhoeller
Copy link
Contributor

This really depends on whether cookie values are seen as input parameters (along the lines of query parameters or path variables) or rather as supplementary values that can simply be ignored if invalid. I suppose we could easily allow for the latter through a configuration setting on CookieLocaleResolver.

@jhoeller jhoeller added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Apr 30, 2019
@jhoeller jhoeller added this to the 5.1.7 milestone Apr 30, 2019
@darioseidl
Copy link
Author

I suppose we could easily allow for the latter through a configuration setting on CookieLocaleResolver

That would be great. In our case, the frontend would be responsible for setting a correct value but if it doesn't, the backend should simply fallback to defaults.

If in the CookieLocaleResolver the throw new IllegalStateException could simply be skipped, it would proceed to this part

request.setAttribute(LOCALE_REQUEST_ATTRIBUTE_NAME, 
        (locale != null ? locale : determineDefaultLocale(request)));
request.setAttribute(TIME_ZONE_REQUEST_ATTRIBUTE_NAME,
        (timeZone != null ? timeZone : determineDefaultTimeZone(request)));

which would be correctly setting the default locale and timezone.

@jhoeller
Copy link
Contributor

Alright, I've introduced a corresponding rejectInvalidCookies flag which is on by default but can be turned off for lenient handling of parse failures.

@darioseidl
Copy link
Author

Thanks for the quick fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants