Corrected port number validation range (#304)

* Corrected port number validation range * added UT for issue 291 * added ut dependency file formatting * correct param in config_test_env.properties * removed incorrect flags from test case config files * added negative test cases * removed properties file and changes in test case Co-authored-by: Harsh Bafna <harshbafna619@gmail.com>
pytorch · May 21, 2020 · 7844a11 · 7844a11
1 parent b32a15c
commit 7844a11
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 4 deletions.
diff --git a/frontend/server/src/main/java/org/pytorch/serve/util/Connector.java b/frontend/server/src/main/java/org/pytorch/serve/util/Connector.java
@@ -105,7 +105,7 @@ public static Connector parse(String binding, boolean management) {
         } else {
             port = Integer.parseInt(listeningPort);
         }
-        if (port >= Short.MAX_VALUE) {
+        if (port >= Short.MAX_VALUE * 2 + 1) {
             throw new IllegalArgumentException("Invalid port number: " + binding);
         }
         return new Connector(port, false, host, String.valueOf(port), ssl, management);

diff --git a/frontend/server/src/test/java/org/pytorch/serve/ModelServerTest.java b/frontend/server/src/test/java/org/pytorch/serve/ModelServerTest.java
@@ -26,6 +26,7 @@
 import java.util.List;
 import java.util.Properties;
 import java.util.concurrent.CountDownLatch;
+import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.IOUtils;
 import org.pytorch.serve.http.DescribeModelResponse;
 import org.pytorch.serve.http.ErrorResponse;
@@ -1366,6 +1367,61 @@ public void testUnregisterModelFailure() throws InterruptedException {
         TestUtils.unregisterModel(channel, "noopversioned", "1.2.1", false);
     }
 
+    @Test(
+            alwaysRun = true,
+            dependsOnMethods = {"testUnregisterModelFailure"})
+    private void testTSValidPort()
+            throws InterruptedException, InvalidSnapshotException, GeneralSecurityException,
+                    IOException {
+        //  test case for verifying port range refer https://github.com/pytorch/serve/issues/291
+        server.stop();
+        FileUtils.deleteQuietly(new File(System.getProperty("LOG_LOCATION"), "config"));
+        configManager.setProperty("inference_address", "https://127.0.0.1:42523");
+        server = new ModelServer(configManager);
+        server.start();
+
+        Channel channel = null;
+        Channel managementChannel = null;
+        for (int i = 0; i < 5; ++i) {
+            channel = TestUtils.connect(false, configManager);
+            if (channel != null) {
+                break;
+            }
+            Thread.sleep(100);
+        }
+
+        for (int i = 0; i < 5; ++i) {
+            managementChannel = TestUtils.connect(true, configManager);
+            if (managementChannel != null) {
+                break;
+            }
+            Thread.sleep(100);
+        }
+
+        Assert.assertNotNull(channel, "Failed to connect to inference port.");
+        Assert.assertNotNull(managementChannel, "Failed to connect to management port.");
+
+        testPing();
+    }
+
+    @Test(
+            alwaysRun = true,
+            dependsOnMethods = {"testTSValidPort"})
+    private void testTSInvalidPort() {
+        //  test case for verifying port range refer https://github.com/pytorch/serve/issues/291
+        //  invalid port test
+        server.stop();
+        FileUtils.deleteQuietly(new File(System.getProperty("LOG_LOCATION"), "config"));
+        configManager.setProperty("inference_address", "https://127.0.0.1:65536");
+        server = new ModelServer(configManager);
+        try {
+            server.start();
+        } catch (Exception e) {
+            Assert.assertEquals(e.getClass(), IllegalArgumentException.class);
+            Assert.assertEquals(e.getMessage(), "Invalid port number: https://127.0.0.1:65536");
+        }
+    }
+
     private void testLoadModel(String url, String modelName) throws InterruptedException {
         Channel channel = TestUtils.getManagementChannel(configManager);
         TestUtils.setResult(null);

diff --git a/frontend/server/src/test/resources/config.properties b/frontend/server/src/test/resources/config.properties
@@ -25,7 +25,6 @@ certificate_file=src/test/resources/certs.pem
 # max_response_size=6553500
 max_request_size=10485760
 # blacklist_env_vars=.*USERNAME.*|.*PASSWORD.*
-# enable_env_vars_config=false
 # decode_input_request=true
 enable_envvars_config=true
 # default_service_handler=/path/to/service.py:handle
diff --git a/frontend/server/src/test/resources/config_test_env.properties b/frontend/server/src/test/resources/config_test_env.properties
@@ -24,6 +24,5 @@ certificate_file=src/test/resources/certs.pem
 # max_response_size=6553500
 max_request_size=10485760
 # blacklist_env_vars=.*USERNAME.*|.*PASSWORD.*
-# enable_env_vars_config=false
-# decode_input_request=true
 # enable_envvars_config=false
+# decode_input_request=true