From 0ecde06c0ae0776dcf9e9adec6812f5826aece51 Mon Sep 17 00:00:00 2001 From: Laurent SCHOELENS Date: Thu, 30 Mar 2023 16:12:19 +0200 Subject: [PATCH] [GH-2284] fix commitSession() called multiple times in requestDispatcher.include --- .../web/http/SessionRepositoryFilter.java | 14 +++++++--- .../http/SessionRepositoryFilterTests.java | 26 +++++++++++++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/spring-session-core/src/main/java/org/springframework/session/web/http/SessionRepositoryFilter.java b/spring-session-core/src/main/java/org/springframework/session/web/http/SessionRepositoryFilter.java index 46af1c4d0..25d57166a 100644 --- a/spring-session-core/src/main/java/org/springframework/session/web/http/SessionRepositoryFilter.java +++ b/spring-session-core/src/main/java/org/springframework/session/web/http/SessionRepositoryFilter.java @@ -205,6 +205,8 @@ private final class SessionRepositoryRequestWrapper extends HttpServletRequestWr private boolean requestedSessionInvalidated; + private boolean hasCommitedInInclude; + private SessionRepositoryRequestWrapper(HttpServletRequest request, HttpServletResponse response) { super(request); this.response = response; @@ -338,7 +340,7 @@ public String getRequestedSessionId() { @Override public RequestDispatcher getRequestDispatcher(String path) { RequestDispatcher requestDispatcher = super.getRequestDispatcher(path); - return new SessionCommittingRequestDispatcher(requestDispatcher); + return new SessionCommittingRequestDispatcher(requestDispatcher, this); } private S getRequestedSession() { @@ -397,8 +399,11 @@ private final class SessionCommittingRequestDispatcher implements RequestDispatc private final RequestDispatcher delegate; - SessionCommittingRequestDispatcher(RequestDispatcher delegate) { + private final SessionRepositoryRequestWrapper wrapper; + + SessionCommittingRequestDispatcher(RequestDispatcher delegate, SessionRepositoryRequestWrapper wrapper) { this.delegate = delegate; + this.wrapper = wrapper; } @Override @@ -408,7 +413,10 @@ public void forward(ServletRequest request, ServletResponse response) throws Ser @Override public void include(ServletRequest request, ServletResponse response) throws ServletException, IOException { - SessionRepositoryRequestWrapper.this.commitSession(); + if (!this.wrapper.hasCommitedInInclude) { + SessionRepositoryRequestWrapper.this.commitSession(); + this.wrapper.hasCommitedInInclude = true; + } this.delegate.include(request, response); } diff --git a/spring-session-core/src/test/java/org/springframework/session/web/http/SessionRepositoryFilterTests.java b/spring-session-core/src/test/java/org/springframework/session/web/http/SessionRepositoryFilterTests.java index 523d8ea1b..7137b1333 100644 --- a/spring-session-core/src/test/java/org/springframework/session/web/http/SessionRepositoryFilterTests.java +++ b/spring-session-core/src/test/java/org/springframework/session/web/http/SessionRepositoryFilterTests.java @@ -1320,6 +1320,32 @@ public void doFilter(HttpServletRequest wrappedRequest) { assertThat(bindingListener.getCounter()).isEqualTo(1); } + @Test // gh-2284 + void doFilterIncludeCommitSessionOnce() throws Exception { + MapSession session = this.sessionRepository.createSession(); + this.sessionRepository.save(session); + SessionRepository sessionRepository = spy(this.sessionRepository); + setSessionCookie(session.getId()); + + given(sessionRepository.findById(session.getId())).willReturn(session); + + this.filter = new SessionRepositoryFilter<>(sessionRepository); + + doFilter(new DoInFilter() { + @Override + public void doFilter(HttpServletRequest wrappedRequest, HttpServletResponse wrappedResponse) + throws IOException, ServletException { + String id = wrappedRequest.getSession().getId(); + wrappedRequest.getRequestDispatcher("/").include(wrappedRequest, wrappedResponse); + assertThat(SessionRepositoryFilterTests.this.sessionRepository.findById(id)).isNotNull(); + wrappedRequest.getRequestDispatcher("/").include(wrappedRequest, wrappedResponse); + verify(sessionRepository).findById(session.getId()); + verify(sessionRepository).save(session); + verifyNoMoreInteractions(sessionRepository); + } + }); + } + // --- helper methods private void assertNewSession() {