Be stricter about request property parsing (and test this)

2 files changed, 16 insertions, 3 deletions

diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java
index 36bec39d10c..3158c4e31be 100644
--- a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java
+++ b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java
@@ -995,11 +995,15 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler {
     // ------------------------------------------------ Helpers ------------------------------------------------
 
     private static Optional<String> getProperty(HttpRequest request, String name) {
+        if ( ! request.parameters().containsKey(name))
+            return Optional.empty();
+
         List<String> values = request.parameters().get(name);
-        if (values != null && values.size() != 0)
-            return Optional.ofNullable(values.get(values.size() - 1));
+        String value;
+        if (values == null || values.isEmpty() || (value = values.get(values.size() - 1)) == null || value.isEmpty())
+            throw new IllegalArgumentException("Expected non-empty value for request property '" + name + "'");
 
-        return Optional.empty();
+        return Optional.of(value);
     }
 
     private static <T> Optional<T> getProperty(HttpRequest request, String name, Parser<T> parser) {
diff --git a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java
index da09d076728..e10b162078a 100644
--- a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java
+++ b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java
@@ -384,6 +384,15 @@ public class DocumentV1ApiTest {
                        "}", response.readAll());
         assertEquals(200, response.getStatus());
 
+        // GET with empty route is a 400
+        access.session.expect((__, ___) -> { throw new AssertionError("Not supposed to happen"); });
+        response = driver.sendRequest("http://localhost/document/v1/space/music/number/1/two?route=", DELETE);
+        assertSameJson("{" +
+                       "  \"pathId\": \"/document/v1/space/music/number/1/two\"," +
+                       "  \"message\": \"Expected non-empty value for request property 'route'\"" +
+                       "}", response.readAll());
+        assertEquals(400, response.getStatus());
+
         // GET with non-existent cluster is a 400
         access.session.expect((__, ___) -> { throw new AssertionError("Not supposed to happen"); });
         response = driver.sendRequest("http://localhost/document/v1/space/music/number/1/two?cluster=throw-me");