[ 
https://issues.apache.org/jira/browse/WW-4734?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15828519#comment-15828519
 ] 

James Chaplin commented on WW-4734:
-----------------------------------

Hello Lukasz. 

The refactored *I18nInterceptor.java* appears to work as expected now (tried 
both using the source directly to replace the default i18n interceptor for 
2.5.8, and using "struts2-core-2.5.9-20170115.171851-15" core jar).  *Thanks* 
for looking into the bug and producing a fix for it.

*Note 1*: JavaDoc comment for I18nInterceptor.java states: _storage (optional) 
-_, but should be _localeStorage (optional) -_ to match the setter 
(corresponding to _<param name="localeStorage">session</param>_ in the 
configuration).

*Note 2*: According to the old 2.5.5 code it appears that if _Storage.COOKIE_ 
was configured, _storeLocale()_ stored it in the cookie then set storage type 
to _Storage.SESSION_ (which caused it also to be stored in session if 
available).  After that the i18n interceptor would be stuck in 
_Storage.SESSION_ (but lookups would still find things programmatically stored 
by cookie ... I think).  I don't think that wrinkle would negatively impact any 
2.5.x apps using _Storage.COOKIE_ with the latest implementation, but I figured 
it was worth mentioning.

In any event the recent I18nInterceptor refactoring seems cleaner than the 
2.5.5 and 2.5.8 revisions (the configured storage level for the interceptor 
remains invariant over invocations), and appears to work for _Storage.SESSION_ 
as expected.  If I note any problem later on I will advise.  Thanks again.

> I18Interceptor ignores session or cookie Locale after first lookup failure
> --------------------------------------------------------------------------
>
>                 Key: WW-4734
>                 URL: https://issues.apache.org/jira/browse/WW-4734
>             Project: Struts 2
>          Issue Type: Bug
>          Components: Core Interceptors
>    Affects Versions: 2.5.8
>            Reporter: James Chaplin
>            Assignee: Lukasz Lenart
>              Labels: interceptors
>             Fix For: 2.5.9
>
>
> In Struts 2.5.5 and prior 2.5.x/2.3.x, the I18nInterceptor honoured the 
> locale state set programmatically via session (e.g. 
> session.put(I18nInterceptor.DEFAULT_SESSION_ATTRIBUTE, localeForUser); ).
> Struts 2.5.8 ignores a locale state set programmatically via session (or at 
> least it does so after the 1st failed lookup).
> *Issue cause: Changes in 2.5.8 storeLocale()/readStoredLocale() behaviour 
> causes "storage == Storage.NONE" after 1st lookup failure, and 
> i18nInterceptor will never again check session/cookie scopes for a locale.  
> Appears to have been introduced with changes for WW-4722.
> *Bug elements:
>   1) readStoredLocale() - Never checks session/cookie level for a saved 
> locale (after first lookup failure sets "storage = Storage.NONE").
>   2) readStoredLocale() - The "if block" checks are inverted.  When "storage 
> == Storage.COOKIE" it checks session, when "storage == Storage.SESSION" it 
> checks cookie (appears this was addressed in update to WW-4722 on 2017/01/11).
>   3) LocaleFinder/saveLocale() - No longer provides default lookup at session 
> level, no longer preserves the storage level where the lookup succeded.
> *Suggested remedy:
>   1) Restore logic equivalent to 2.5.5 and earlier that will always check 
> session and cookie scopes for Locale, irrespective of what the current i18n 
> interceptor instance's storage value is set to.
>      Change readStoredLocale() to check all scopes, restore storage scope 
> state to LocaleFinder and calls to saveLocale().  Use LocaleFilender's scope 
> state (tracking immediate request's locale storage level) during request 
> processing (and if possible, leave i18n interceptor's scope fixed/unchanged).
>   2) Add logic to I18nInterceptor that preserves the initially configured 
> storage type/scope for the lifetime of the i18n interceptor.  This could be 
> done by adding a new protected member to I18nInterceptor (e.g. protected 
> configuredStorage), which gets initialized similarly to "storage" but only 
> modified in the initial setLocaleStorage() call (so its value stays intact 
> until explicitly changed by configuration).
> Either way it appears the I18nInterceptor needs a mechanism to ensure that it 
> will always look for Locale at the session/cookie level (or at the very least 
> the level that was initially configured for the interceptor), irrespective of 
> what the current storage value is set to.  Without such logic the i18n 
> interceptor stops looking for anything other than the request/invocation 
> context level (after the first lookup failure, irrespective of original 
> storage setting).  Tracking the configured storage scope (global) and the 
> immediate request's scope (local) separately might be appropriate.
> *Note: API documentation for I18nInterceptor "storage" parameter appears 
> incorrect as well.  The new configuration parameter in 2.5.8 should indicate 
> "localeStorage" for configuring the locale storage parameter (it indicates 
> "storage" currently, but that fails as it doesn't match the setter's name).



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to