[
https://issues.apache.org/jira/browse/WW-4741?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16346792#comment-16346792
]
ASF GitHub Bot commented on WW-4741:
------------------------------------
yasserzamani closed pull request #207: WW-4741: Do not create session
URL: https://github.com/apache/struts/pull/207
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/core/src/main/java/org/apache/struts2/interceptor/I18nInterceptor.java
b/core/src/main/java/org/apache/struts2/interceptor/I18nInterceptor.java
index 502c21bd0..682bbe6d7 100644
--- a/core/src/main/java/org/apache/struts2/interceptor/I18nInterceptor.java
+++ b/core/src/main/java/org/apache/struts2/interceptor/I18nInterceptor.java
@@ -281,15 +281,17 @@ public Locale find() {
@Override
public Locale store(ActionInvocation invocation, Locale locale) {
- //save it in session
- Map<String, Object> session =
invocation.getInvocationContext().getSession();
+ HttpSession session =
ServletActionContext.getRequest().getSession(false);
if (session != null) {
- String sessionId =
ServletActionContext.getRequest().getSession().getId();
+ String sessionId = session.getId();
synchronized (sessionId.intern()) {
- session.put(attributeName, locale);
+
invocation.getInvocationContext().getSession().put(attributeName, locale);
}
+ } else {
+ LOG.debug("session creation avoided as it doesn't exist
already");
}
+
return locale;
}
@@ -298,19 +300,15 @@ public Locale read(ActionInvocation invocation) {
Locale locale = null;
LOG.debug("Checks session for saved locale");
- Map<String, Object> session =
invocation.getInvocationContext().getSession();
+ HttpSession session =
ServletActionContext.getRequest().getSession(false);
if (session != null) {
- //[WW-4741] Do not force session creation while this is a read
operation
- HttpSession httpSession =
ServletActionContext.getRequest().getSession(false);
- if(null != httpSession) {
- String sessionId = httpSession.getId();
- synchronized (sessionId.intern()) {
- Object sessionLocale = session.get(attributeName);
- if (sessionLocale != null && sessionLocale instanceof
Locale) {
- locale = (Locale) sessionLocale;
- LOG.debug("Applied session locale: {}", locale);
- }
+ String sessionId = session.getId();
+ synchronized (sessionId.intern()) {
+ Object sessionLocale =
invocation.getInvocationContext().getSession().get(attributeName);
+ if (sessionLocale != null && sessionLocale instanceof
Locale) {
+ locale = (Locale) sessionLocale;
+ LOG.debug("Applied session locale: {}", locale);
}
}
}
diff --git
a/core/src/test/java/org/apache/struts2/interceptor/I18nInterceptorTest.java
b/core/src/test/java/org/apache/struts2/interceptor/I18nInterceptorTest.java
index 0a1942fb7..fcc0dd11b 100644
--- a/core/src/test/java/org/apache/struts2/interceptor/I18nInterceptorTest.java
+++ b/core/src/test/java/org/apache/struts2/interceptor/I18nInterceptorTest.java
@@ -31,6 +31,7 @@
import org.easymock.EasyMock;
import org.easymock.IArgumentMatcher;
import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.mock.web.MockHttpSession;
import javax.servlet.http.Cookie;
import javax.servlet.http.HttpServletResponse;
@@ -45,14 +46,41 @@
private ActionInvocation mai;
private ActionContext ac;
private Map session;
+ private MockHttpServletRequest request;
public void testEmptyParamAndSession() throws Exception {
interceptor.intercept(mai);
}
- public void testNoSession() throws Exception {
- ac.setSession(null);
- interceptor.intercept(mai);
+ public void testNoSessionNoLocale() throws Exception {
+ request.setSession(null);
+ try {
+ interceptor.intercept(mai);
+ assertTrue(true);
+ } catch (Exception ignore) {
+ fail("Shouldn't throw any exception!");
+ }
+
+ assertFalse("should have been removed",
+
mai.getInvocationContext().getParameters().get(I18nInterceptor.DEFAULT_PARAMETER).isDefined());
+ assertNull("should not be created", request.getSession(false));
+ assertNull("should not be stored here",
session.get(I18nInterceptor.DEFAULT_SESSION_ATTRIBUTE));
+ }
+
+ public void testNoSessionButLocale() throws Exception {
+ prepare(I18nInterceptor.DEFAULT_PARAMETER, "da_DK"); //prevents
shouldStore to being false
+ request.setSession(null);
+ try {
+ interceptor.intercept(mai);
+ assertTrue(true);
+ } catch (Exception ignore) {
+ fail("Shouldn't throw any exception!");
+ }
+
+ assertFalse("should have been removed",
+
mai.getInvocationContext().getParameters().get(I18nInterceptor.DEFAULT_PARAMETER).isDefined());
+ assertNull("should not be created", request.getSession(false));
+ assertNull("should not be stored here",
session.get(I18nInterceptor.DEFAULT_SESSION_ATTRIBUTE));
}
public void testDefaultLocale() throws Exception {
@@ -235,7 +263,9 @@ public void setUp() throws Exception {
ac = new ActionContext(ctx);
ServletActionContext.setContext(ac);
- ServletActionContext.setRequest(new MockHttpServletRequest());
+ request = new MockHttpServletRequest();
+ request.setSession(new MockHttpSession());
+ ServletActionContext.setRequest(request);
Action action = new Action() {
public String execute() throws Exception {
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> Http Sessions forcefully created for all requests using I18nInterceptor with
> default Storage value.
> ---------------------------------------------------------------------------------------------------
>
> Key: WW-4741
> URL: https://issues.apache.org/jira/browse/WW-4741
> Project: Struts 2
> Issue Type: Bug
> Affects Versions: 2.5.10
> Reporter: Adam Greenfield
> Assignee: Lukasz Lenart
> Priority: Major
> Fix For: 2.5.15
>
>
> Changes made in WW-4730 for store and read functions cause an httpSession to
> be created for every request that uses I18nInterceptor when storage =
> Storage.SESSION.
> Current code checks for
> {noformat}Map<String, Object> session =
> invocation.getInvocationContext().getSession(){noformat}
> to be null and then calls
> {noformat}ServletActionContext.getRequest().getSession(){noformat}
> (notice how the second one references the HttpServletRequest. The
> HttpServletRequest Session and and the InvocationContext session are
> different. The request's session can be null, even if the
> InvocationContext's session is not).
> Calling .getSession() in this manner forcefully creates a session.
> An appropriate check here might be
> {noformat}HttpSession httpSession =
> ServletActionContext.getRequest().getSession(false);
> if(httpSession != null) {
> ... // get sessionId and synchronize on it
> }
> {noformat}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
