gemmellr commented on code in PR #4441:
URL: https://github.com/apache/activemq-artemis/pull/4441#discussion_r1178936369
##########
artemis-web/src/test/java/org/apache/activemq/cli/test/WebServerComponentTest.java:
##########
@@ -283,6 +298,40 @@ public void simpleSecureServer() throws Exception {
Assert.assertFalse(webServerComponent.isStarted());
}
+
+ @Test
+ public void testSimpleSecureServerWithSniHostCheckEnabled() throws
Exception {
+ testSimpleSecureServerWithSniHostCheck(true);
+ }
+
+ @Test
+ public void testSimpleSecureServerWithSniHostCheckDisabled() throws
Exception {
+ testSimpleSecureServerWithSniHostCheck(false);
+ }
Review Comment:
Do we want to test the default behaviour is 'enabled' (i.e change the
boolean to Boolean and pass null as well)?
Currently nothing looks to be checking the default, since these 2 tests only
verify it succeeds/fails using the IP address when the setting was _explicitly_
disabled or enabled.
##########
artemis-dto/src/main/java/org/apache/activemq/artemis/dto/BindingDTO.java:
##########
@@ -181,6 +187,22 @@ public void addApp(AppDTO app) {
apps.add(app);
}
+ public Boolean getSniHostCheck() {
+ return sniHostCheck;
+ }
+
+ public void setSniHostCheck(Boolean sniHostCheck) {
+ this.sniHostCheck = sniHostCheck;
+ }
+
+ public Boolean getSniRequired() {
+ return sniRequired;
+ }
+
+ public void setSniRequired(Boolean sniRequired) {
+ this.sniRequired = sniRequired;
+ }
+
Review Comment:
org.apache.activemq.artemis.dto.test.BindingDTOTest.testDefault() should be
updated to check these are null by default since the new default behaviour
depends on it.
##########
artemis-web/src/test/java/org/apache/activemq/cli/test/WebServerComponentTest.java:
##########
@@ -215,11 +221,11 @@ public void testComponentStopStartBehavior() throws
Exception {
Assert.assertFalse(webServerComponent.isStarted());
}
- @Test
- public void simpleSecureServer() throws Exception {
+ private WebServerComponent startSimpleSecureServer(String keyStorePath,
String keyStorePassword, Boolean sniHostCheck) throws Exception {
BindingDTO bindingDTO = new BindingDTO();
bindingDTO.uri = "https://localhost:0";;
bindingDTO.keyStorePath = "./src/test/resources/server.keystore";
+ bindingDTO.setSniHostCheck(sniHostCheck);
Review Comment:
might be nice to only set it if non-null?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
