From dc2947c52df18d5e99cad03383f7d6ba13d031fd Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Fri, 29 Apr 2022 10:41:16 +0100 Subject: [PATCH 1/2] Ignore invalid connect frame Closes gh-28443 --- .../broker/SimpleBrokerMessageHandler.java | 8 +++++- .../stomp/StompBrokerRelayMessageHandler.java | 8 +++++- .../StompBrokerRelayMessageHandlerTests.java | 26 ++++++++++++++++++- 3 files changed, 39 insertions(+), 3 deletions(-) diff --git a/spring-messaging/src/main/java/org/springframework/messaging/simp/broker/SimpleBrokerMessageHandler.java b/spring-messaging/src/main/java/org/springframework/messaging/simp/broker/SimpleBrokerMessageHandler.java index bcfe2dc3fe..7f795eb67e 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/simp/broker/SimpleBrokerMessageHandler.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/simp/broker/SimpleBrokerMessageHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 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. @@ -306,6 +306,12 @@ public class SimpleBrokerMessageHandler extends AbstractBrokerMessageHandler { else if (SimpMessageType.CONNECT.equals(messageType)) { logMessage(message); if (sessionId != null) { + if (this.sessions.get(sessionId) != null) { + if (logger.isWarnEnabled()) { + logger.warn("Ignoring CONNECT in session " + sessionId + ". Already connected."); + } + return; + } long[] heartbeatIn = SimpMessageHeaderAccessor.getHeartbeat(headers); long[] heartbeatOut = getHeartbeatValue(); Principal user = SimpMessageHeaderAccessor.getUser(headers); diff --git a/spring-messaging/src/main/java/org/springframework/messaging/simp/stomp/StompBrokerRelayMessageHandler.java b/spring-messaging/src/main/java/org/springframework/messaging/simp/stomp/StompBrokerRelayMessageHandler.java index f756b0b1f9..a0285a2d2f 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/simp/stomp/StompBrokerRelayMessageHandler.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/simp/stomp/StompBrokerRelayMessageHandler.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. @@ -552,6 +552,12 @@ public class StompBrokerRelayMessageHandler extends AbstractBrokerMessageHandler } if (StompCommand.CONNECT.equals(command) || StompCommand.STOMP.equals(command)) { + if (this.connectionHandlers.get(sessionId) != null) { + if (logger.isWarnEnabled()) { + logger.warn("Ignoring CONNECT in session " + sessionId + ". Already connected."); + } + return; + } if (logger.isDebugEnabled()) { logger.debug(stompAccessor.getShortLogMessage(EMPTY_PAYLOAD)); } diff --git a/spring-messaging/src/test/java/org/springframework/messaging/simp/stomp/StompBrokerRelayMessageHandlerTests.java b/spring-messaging/src/test/java/org/springframework/messaging/simp/stomp/StompBrokerRelayMessageHandlerTests.java index 3fc34442d7..44f52a68b4 100644 --- a/spring-messaging/src/test/java/org/springframework/messaging/simp/stomp/StompBrokerRelayMessageHandlerTests.java +++ b/spring-messaging/src/test/java/org/springframework/messaging/simp/stomp/StompBrokerRelayMessageHandlerTests.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. @@ -260,6 +260,30 @@ class StompBrokerRelayMessageHandlerTests { assertThat(captor.getValue()).isSameAs(message); } + @Test + void alreadyConnected() { + + this.brokerRelay.start(); + + Message connect = connectMessage("sess1", "joe"); + this.brokerRelay.handleMessage(connect); + + assertThat(this.tcpClient.getSentMessages().size()).isEqualTo(2); + + StompHeaderAccessor headers1 = this.tcpClient.getSentHeaders(0); + assertThat(headers1.getCommand()).isEqualTo(StompCommand.CONNECT); + assertThat(headers1.getSessionId()).isEqualTo(StompBrokerRelayMessageHandler.SYSTEM_SESSION_ID); + + StompHeaderAccessor headers2 = this.tcpClient.getSentHeaders(1); + assertThat(headers2.getCommand()).isEqualTo(StompCommand.CONNECT); + assertThat(headers2.getSessionId()).isEqualTo("sess1"); + + this.brokerRelay.handleMessage(connect); + + assertThat(this.tcpClient.getSentMessages().size()).isEqualTo(2); + assertThat(this.outboundChannel.getMessages()).isEmpty(); + } + private Message connectMessage(String sessionId, String user) { StompHeaderAccessor headers = StompHeaderAccessor.create(StompCommand.CONNECT); headers.setSessionId(sessionId); From 83186b689f11f5e6efe7ccc08fdeb92f66fcd583 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 11 May 2022 08:32:06 +0200 Subject: [PATCH 2/2] Refine CachedIntrospectionResults property introspection Closes gh-28445 --- .../beans/CachedIntrospectionResults.java | 26 ++++++++++++------- .../beans/BeanWrapperTests.java | 21 ++++++++++++++- 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/CachedIntrospectionResults.java b/spring-beans/src/main/java/org/springframework/beans/CachedIntrospectionResults.java index 8332045197..bd234eb58f 100644 --- a/spring-beans/src/main/java/org/springframework/beans/CachedIntrospectionResults.java +++ b/spring-beans/src/main/java/org/springframework/beans/CachedIntrospectionResults.java @@ -22,6 +22,7 @@ import java.beans.Introspector; import java.beans.PropertyDescriptor; import java.lang.reflect.Method; import java.lang.reflect.Modifier; +import java.net.URL; import java.security.ProtectionDomain; import java.util.Collections; import java.util.HashSet; @@ -292,10 +293,12 @@ public final class CachedIntrospectionResults { // Only allow all name variants of Class properties continue; } - if (pd.getWriteMethod() == null && pd.getPropertyType() != null && - (ClassLoader.class.isAssignableFrom(pd.getPropertyType()) || - ProtectionDomain.class.isAssignableFrom(pd.getPropertyType()))) { - // Ignore ClassLoader and ProtectionDomain read-only properties - no need to bind to those + if (URL.class == beanClass && "content".equals(pd.getName())) { + // Only allow URL attribute introspection, not content resolution + continue; + } + if (pd.getWriteMethod() == null && isInvalidReadOnlyPropertyType(pd.getPropertyType())) { + // Ignore read-only properties such as ClassLoader - no need to bind to those continue; } if (logger.isTraceEnabled()) { @@ -344,10 +347,8 @@ public final class CachedIntrospectionResults { // GenericTypeAwarePropertyDescriptor leniently resolves a set* write method // against a declared read method, so we prefer read method descriptors here. pd = buildGenericTypeAwarePropertyDescriptor(beanClass, pd); - if (pd.getWriteMethod() == null && pd.getPropertyType() != null && - (ClassLoader.class.isAssignableFrom(pd.getPropertyType()) || - ProtectionDomain.class.isAssignableFrom(pd.getPropertyType()))) { - // Ignore ClassLoader and ProtectionDomain read-only properties - no need to bind to those + if (pd.getWriteMethod() == null && isInvalidReadOnlyPropertyType(pd.getPropertyType())) { + // Ignore read-only properties such as ClassLoader - no need to bind to those continue; } this.propertyDescriptors.put(pd.getName(), pd); @@ -379,8 +380,7 @@ public final class CachedIntrospectionResults { if (Modifier.isStatic(method.getModifiers()) || method.getDeclaringClass() == Object.class || method.getDeclaringClass() == Class.class || method.getParameterCount() > 0 || method.getReturnType() == void.class || - ClassLoader.class.isAssignableFrom(method.getReturnType()) || - ProtectionDomain.class.isAssignableFrom(method.getReturnType())) { + isInvalidReadOnlyPropertyType(method.getReturnType())) { return false; } try { @@ -393,6 +393,12 @@ public final class CachedIntrospectionResults { } } + private boolean isInvalidReadOnlyPropertyType(@Nullable Class returnType) { + return (returnType != null && (AutoCloseable.class.isAssignableFrom(returnType) || + ClassLoader.class.isAssignableFrom(returnType) || + ProtectionDomain.class.isAssignableFrom(returnType))); + } + BeanInfo getBeanInfo() { return this.beanInfo; diff --git a/spring-beans/src/test/java/org/springframework/beans/BeanWrapperTests.java b/spring-beans/src/test/java/org/springframework/beans/BeanWrapperTests.java index ab154ea3c4..dd79e71cb1 100644 --- a/spring-beans/src/test/java/org/springframework/beans/BeanWrapperTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/BeanWrapperTests.java @@ -25,6 +25,7 @@ import org.junit.jupiter.api.Test; import org.springframework.beans.testfixture.beans.TestBean; import org.springframework.core.OverridingClassLoader; import org.springframework.core.io.DefaultResourceLoader; +import org.springframework.core.io.UrlResource; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; @@ -153,7 +154,7 @@ class BeanWrapperTests extends AbstractPropertyAccessorTests { } @Test - void propertyDescriptors() { + void propertyDescriptors() throws Exception { TestBean target = new TestBean(); target.setSpouse(new TestBean()); BeanWrapper accessor = createAccessor(target); @@ -182,11 +183,29 @@ class BeanWrapperTests extends AbstractPropertyAccessorTests { assertThat(accessor.isReadableProperty("class.package")).isFalse(); assertThat(accessor.isReadableProperty("class.module")).isFalse(); assertThat(accessor.isReadableProperty("class.classLoader")).isFalse(); + assertThat(accessor.isReadableProperty("class.name")).isTrue(); + assertThat(accessor.isReadableProperty("class.simpleName")).isTrue(); assertThat(accessor.isReadableProperty("classLoader")).isTrue(); assertThat(accessor.isWritableProperty("classLoader")).isTrue(); OverridingClassLoader ocl = new OverridingClassLoader(getClass().getClassLoader()); accessor.setPropertyValue("classLoader", ocl); assertThat(accessor.getPropertyValue("classLoader")).isSameAs(ocl); + + accessor = createAccessor(new UrlResource("https://spring.io")); + + assertThat(accessor.isReadableProperty("class.package")).isFalse(); + assertThat(accessor.isReadableProperty("class.module")).isFalse(); + assertThat(accessor.isReadableProperty("class.classLoader")).isFalse(); + assertThat(accessor.isReadableProperty("class.name")).isTrue(); + assertThat(accessor.isReadableProperty("class.simpleName")).isTrue(); + assertThat(accessor.isReadableProperty("URL.protocol")).isTrue(); + assertThat(accessor.isReadableProperty("URL.host")).isTrue(); + assertThat(accessor.isReadableProperty("URL.port")).isTrue(); + assertThat(accessor.isReadableProperty("URL.file")).isTrue(); + assertThat(accessor.isReadableProperty("URL.content")).isFalse(); + assertThat(accessor.isReadableProperty("inputStream")).isFalse(); + assertThat(accessor.isReadableProperty("filename")).isTrue(); + assertThat(accessor.isReadableProperty("description")).isTrue(); } @Test