From f004bb1b64cd9f39d58adc181607a1d5f3034b29 Mon Sep 17 00:00:00 2001 From: Ivan Zbykovskyi <> Date: Thu, 3 Feb 2022 13:34:02 +0200 Subject: [PATCH 1/3] Add formatting for SockJS GoAway frame Prevents infinite loop for xhr-polling and xhr-streaming transports. See gh-28000 --- .../handler/AbstractHttpSendingTransportHandler.java | 4 +++- .../sockjs/transport/session/AbstractHttpSockJsSession.java | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/AbstractHttpSendingTransportHandler.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/AbstractHttpSendingTransportHandler.java index ef566c7f8db..73aac7449e4 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/AbstractHttpSendingTransportHandler.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/AbstractHttpSendingTransportHandler.java @@ -79,9 +79,11 @@ public abstract class AbstractHttpSendingTransportHandler extends AbstractTransp if (logger.isDebugEnabled()) { logger.debug("Connection already closed (but not removed yet) for " + sockJsSession); } + SockJsFrameFormat frameFormat = this.getFrameFormat(request); SockJsFrame frame = SockJsFrame.closeFrameGoAway(); + String formattedFrame = frameFormat.format(frame); try { - response.getBody().write(frame.getContentBytes()); + response.getBody().write(formattedFrame.getBytes(SockJsFrame.CHARSET)); } catch (IOException ex) { throw new SockJsException("Failed to send " + frame, sockJsSession.getId(), ex); diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/AbstractHttpSockJsSession.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/AbstractHttpSockJsSession.java index a3d492ecf56..ab90363c7f1 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/AbstractHttpSockJsSession.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/AbstractHttpSockJsSession.java @@ -257,7 +257,8 @@ public abstract class AbstractHttpSockJsSession extends AbstractSockJsSession { synchronized (this.responseLock) { try { if (isClosed()) { - response.getBody().write(SockJsFrame.closeFrameGoAway().getContentBytes()); + String formattedFrame = frameFormat.format(SockJsFrame.closeFrameGoAway()); + response.getBody().write(formattedFrame.getBytes(SockJsFrame.CHARSET)); return; } this.response = response; From 11cb93823270e8c7003c6bf21909bef0601f062d Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Mon, 14 Feb 2022 16:34:40 +0000 Subject: [PATCH 2/3] Polishing contribution Closes gh-28000 --- .../AbstractHttpSendingTransportHandler.java | 32 +++++++++---------- .../session/AbstractHttpSockJsSession.java | 2 +- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/AbstractHttpSendingTransportHandler.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/AbstractHttpSendingTransportHandler.java index 73aac7449e4..9a2f75d1ba0 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/AbstractHttpSendingTransportHandler.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/AbstractHttpSendingTransportHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -79,15 +79,7 @@ public abstract class AbstractHttpSendingTransportHandler extends AbstractTransp if (logger.isDebugEnabled()) { logger.debug("Connection already closed (but not removed yet) for " + sockJsSession); } - SockJsFrameFormat frameFormat = this.getFrameFormat(request); - SockJsFrame frame = SockJsFrame.closeFrameGoAway(); - String formattedFrame = frameFormat.format(frame); - try { - response.getBody().write(formattedFrame.getBytes(SockJsFrame.CHARSET)); - } - catch (IOException ex) { - throw new SockJsException("Failed to send " + frame, sockJsSession.getId(), ex); - } + writeFrame(SockJsFrame.closeFrameGoAway(), request, response, sockJsSession); } else if (!sockJsSession.isActive()) { if (logger.isTraceEnabled()) { @@ -99,13 +91,19 @@ public abstract class AbstractHttpSendingTransportHandler extends AbstractTransp if (logger.isDebugEnabled()) { logger.debug("Another " + getTransportType() + " connection still open for " + sockJsSession); } - String formattedFrame = getFrameFormat(request).format(SockJsFrame.closeFrameAnotherConnectionOpen()); - try { - response.getBody().write(formattedFrame.getBytes(SockJsFrame.CHARSET)); - } - catch (IOException ex) { - throw new SockJsException("Failed to send " + formattedFrame, sockJsSession.getId(), ex); - } + writeFrame(SockJsFrame.closeFrameAnotherConnectionOpen(), request, response, sockJsSession); + } + } + + private void writeFrame(SockJsFrame frame, ServerHttpRequest request, ServerHttpResponse response, + AbstractHttpSockJsSession sockJsSession) { + + String formattedFrame = getFrameFormat(request).format(frame); + try { + response.getBody().write(formattedFrame.getBytes(SockJsFrame.CHARSET)); + } + catch (IOException ex) { + throw new SockJsException("Failed to send " + formattedFrame, sockJsSession.getId(), ex); } } diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/AbstractHttpSockJsSession.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/AbstractHttpSockJsSession.java index ab90363c7f1..04372fb52bf 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/AbstractHttpSockJsSession.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/session/AbstractHttpSockJsSession.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. From ec03e8830ed0edb3cffbda1395c491127c0353b6 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Mon, 14 Feb 2022 20:50:38 +0000 Subject: [PATCH 3/3] Remove path variables from pathWithinMapping Closes gh-27913 --- .../web/servlet/handler/AbstractUrlHandlerMapping.java | 8 ++++---- .../web/servlet/handler/SimpleUrlHandlerMappingTests.java | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractUrlHandlerMapping.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractUrlHandlerMapping.java index 2f7be313ca9..812060198e6 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractUrlHandlerMapping.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractUrlHandlerMapping.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -28,7 +28,6 @@ import javax.servlet.http.HttpServletResponse; import org.springframework.beans.BeansException; import org.springframework.context.ApplicationContext; -import org.springframework.http.server.PathContainer; import org.springframework.http.server.RequestPath; import org.springframework.lang.Nullable; import org.springframework.util.AntPathMatcher; @@ -216,8 +215,9 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping i handler = obtainApplicationContext().getBean(handlerName); } validateHandler(handler, request); - PathContainer pathWithinMapping = pattern.extractPathWithinPattern(path.pathWithinApplication()); - return buildPathExposingHandler(handler, pattern.getPatternString(), pathWithinMapping.value(), null); + String pathWithinMapping = pattern.extractPathWithinPattern(path.pathWithinApplication()).value(); + pathWithinMapping = UrlPathHelper.defaultInstance.removeSemicolonContent(pathWithinMapping); + return buildPathExposingHandler(handler, pattern.getPatternString(), pathWithinMapping, null); } /** diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/SimpleUrlHandlerMappingTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/SimpleUrlHandlerMappingTests.java index d3cace663e3..c47a5e04485 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/SimpleUrlHandlerMappingTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/SimpleUrlHandlerMappingTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -102,7 +102,7 @@ public class SimpleUrlHandlerMappingTests { assertThat(request.getAttribute(PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE)).isEqualTo("/welcome.html"); assertThat(request.getAttribute(BEST_MATCHING_HANDLER_ATTRIBUTE)).isEqualTo(bean); - request = PathPatternsTestUtils.initRequest("GET", "/welcome.x", usePathPatterns); + request = PathPatternsTestUtils.initRequest("GET", "/welcome.x;jsessionid=123", usePathPatterns); chain = getHandler(hm, request); assertThat(chain.getHandler()).isSameAs(otherBean); assertThat(request.getAttribute(PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE)).isEqualTo("welcome.x");