[ 
https://issues.apache.org/jira/browse/ARTEMIS-4245?focusedWorklogId=859372&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-859372
 ]

ASF GitHub Bot logged work on ARTEMIS-4245:
-------------------------------------------

                Author: ASF GitHub Bot
            Created on: 27/Apr/23 10:11
            Start Date: 27/Apr/23 10:11
    Worklog Time Spent: 10m 
      Work Description: 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?





Issue Time Tracking
-------------------

    Worklog Id:     (was: 859372)
    Time Spent: 20m  (was: 10m)

> Expose web SNI settings
> -----------------------
>
>                 Key: ARTEMIS-4245
>                 URL: https://issues.apache.org/jira/browse/ARTEMIS-4245
>             Project: ActiveMQ Artemis
>          Issue Type: Improvement
>            Reporter: Domenico Francesco Bruscino
>            Assignee: Domenico Francesco Bruscino
>            Priority: Major
>          Time Spent: 20m
>  Remaining Estimate: 0h
>
> Expose sniHostCheck and sniRequired settings in the web config.
> {code:xml}
>    <web path="web" rootRedirectLocation="console">
>        <binding uri="http://localhost:8161"; sniHostCheck="false" 
> sniRequired="false">
> ...
> {code}



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to