Use special Param types to avoid 404 when number not parseable

graphhopper · Apr 19, 2020 · f075583 · f075583
1 parent 9a42cd2
commit f075583
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 20 deletions.
diff --git a/web-bundle/src/main/java/com/graphhopper/resources/IsochroneResource.java b/web-bundle/src/main/java/com/graphhopper/resources/IsochroneResource.java
@@ -23,6 +23,8 @@
 import com.graphhopper.util.Parameters;
 import com.graphhopper.util.StopWatch;
 import com.graphhopper.util.shapes.GHPoint;
+import io.dropwizard.jersey.params.IntParam;
+import io.dropwizard.jersey.params.LongParam;
 import io.dropwizard.validation.OneOf;
 import org.hibernate.validator.constraints.Range;
 import org.locationtech.jts.geom.*;
@@ -40,10 +42,7 @@
 import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
 import javax.ws.rs.core.UriInfo;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashMap;
+import java.util.*;
 
 import static com.graphhopper.resources.RouteResource.errorIfLegacyParameters;
 import static com.graphhopper.routing.util.TraversalMode.EDGE_BASED;
@@ -67,15 +66,15 @@ public IsochroneResource(GraphHopper graphHopper, ProfileResolver profileResolve
     }
 
     @GET
-    @Produces({MediaType.APPLICATION_JSON})
+    @Produces(MediaType.APPLICATION_JSON)
     public Response doGet(
             @Context UriInfo uriInfo,
             @QueryParam("profile") String profileName,
-            @QueryParam("buckets") @Range(min=1,max=20) @DefaultValue("1") int nBuckets,
+            @QueryParam("buckets") @Range(min=1,max=20) @DefaultValue("1") IntParam nBuckets,
             @QueryParam("reverse_flow") @DefaultValue("false") boolean reverseFlow,
             @QueryParam("point") @NotNull GHPoint point,
-            @QueryParam("time_limit") @DefaultValue("600") long timeLimitInSeconds,
-            @QueryParam("distance_limit") @DefaultValue("-1") double distanceInMeter,
+            @QueryParam("time_limit") @DefaultValue("600") LongParam timeLimitInSeconds,
+            @QueryParam("distance_limit") @DefaultValue("-1") LongParam distanceInMeter,
             @QueryParam("type") @OneOf({"json","geojson"}) @DefaultValue("json") String respType) {
         StopWatch sw = new StopWatch().start();
 
@@ -112,23 +111,23 @@ public Response doGet(
         ShortestPathTree shortestPathTree = new ShortestPathTree(queryGraph, weighting, reverseFlow, traversalMode);
 
         double limit;
-        if (distanceInMeter > 0) {
-            limit = distanceInMeter;
+        if (distanceInMeter.get() > 0) {
+            limit = distanceInMeter.get();
             shortestPathTree.setDistanceLimit(limit + Math.max(limit * 0.14, 2_000));
         } else {
-            limit = timeLimitInSeconds * 1000;
+            limit = timeLimitInSeconds.get() * 1000;
             shortestPathTree.setTimeLimit(limit + Math.max(limit * 0.14, 200_000));
         }
         ArrayList<Double> zs = new ArrayList<>();
-        for (int i = 0; i < nBuckets; i++) {
-            zs.add(limit / (nBuckets - i));
+        for (int i = 0; i < nBuckets.get(); i++) {
+            zs.add(limit / (nBuckets.get() - i));
         }
 
         final NodeAccess na = queryGraph.getNodeAccess();
         Collection<ConstraintVertex> sites = new ArrayList<>();
         shortestPathTree.search(qr.getClosestNode(), label -> {
             double exploreValue;
-            if (distanceInMeter > 0) {
+            if (distanceInMeter.get() > 0) {
                 exploreValue = label.distance;
             } else {
                 exploreValue = label.time;

diff --git a/web-bundle/src/main/java/com/graphhopper/resources/SPTResource.java b/web-bundle/src/main/java/com/graphhopper/resources/SPTResource.java
@@ -18,6 +18,7 @@
 import com.graphhopper.storage.index.QueryResult;
 import com.graphhopper.util.*;
 import com.graphhopper.util.shapes.GHPoint;
+import io.dropwizard.jersey.params.LongParam;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -70,16 +71,16 @@ public Response doGet(
             @QueryParam("reverse_flow") @DefaultValue("false") boolean reverseFlow,
             @QueryParam("point") @NotNull GHPoint point,
             @QueryParam("columns") String columnsParam,
-            @QueryParam("time_limit") @DefaultValue("600") long timeLimitInSeconds,
-            @QueryParam("distance_limit") @DefaultValue("-1") double distanceInMeter) {
+            @QueryParam("time_limit") @DefaultValue("600") LongParam timeLimitInSeconds,
+            @QueryParam("distance_limit") @DefaultValue("-1") LongParam distanceInMeter) {
         try {
             return executeGet(uriInfo, profileName, reverseFlow, point, columnsParam, timeLimitInSeconds, distanceInMeter);
         } catch (IllegalArgumentException e) {
             return returnBadRequest(e.getMessage());
         }
     }
 
-    private Response executeGet(UriInfo uriInfo, String profileName, boolean reverseFlow, GHPoint point, String columnsParam, long timeLimitInSeconds, double distanceInMeter) {
+    private Response executeGet(UriInfo uriInfo, String profileName, boolean reverseFlow, GHPoint point, String columnsParam, LongParam timeLimitInSeconds, LongParam distanceInMeter) {
         StopWatch sw = new StopWatch().start();
         PMap hintsMap = new PMap();
         RouteResource.initHints(hintsMap, uriInfo.getQueryParameters());
@@ -113,10 +114,10 @@ private Response executeGet(UriInfo uriInfo, String profileName, boolean reverse
         TraversalMode traversalMode = profile.isTurnCosts() ? EDGE_BASED : NODE_BASED;
         ShortestPathTree shortestPathTree = new ShortestPathTree(queryGraph, weighting, reverseFlow, traversalMode);
 
-        if (distanceInMeter > 0) {
-            shortestPathTree.setDistanceLimit(distanceInMeter + Math.max(distanceInMeter * 0.14, 2_000));
+        if (distanceInMeter.get() > 0) {
+            shortestPathTree.setDistanceLimit(distanceInMeter.get() + Math.max(distanceInMeter.get() * 0.14, 2_000));
         } else {
-            double limit = timeLimitInSeconds * 1000;
+            double limit = timeLimitInSeconds.get() * 1000;
             shortestPathTree.setTimeLimit(limit + Math.max(limit * 0.14, 200_000));
         }
 

diff --git a/web/src/test/java/com/graphhopper/http/resources/IsochroneResourceTest.java b/web/src/test/java/com/graphhopper/http/resources/IsochroneResourceTest.java
@@ -225,6 +225,17 @@ public void requestJsonBadType() {
         assertEquals("query param type must be one of [json, geojson]", message);
     }
 
+    @Test
+    public void requestNotANumber() {
+        Response response = clientTarget(app, "/isochrone?profile=fast_car&point=42.531073,1.573792&time_limit=wurst")
+                .request().buildGet().invoke();
+
+        JsonNode json = response.readEntity(JsonNode.class);
+        String message = json.path("message").asText();
+
+        assertEquals("query param time_limit is not a number.", message);
+    }
+
     @Test
     public void requestWithBlockArea() {
         Response rsp = clientTarget(app, "/isochrone")