gis-ops / docker-valhalla

This is our flexible Docker repository for the Valhalla routing engine
MIT License
233 stars 69 forks source link

Empty polygon in avoid_polygons causes container to crash #82

Closed isakengstrom closed 1 year ago

isakengstrom commented 1 year ago

Hi,

I ran into a bug with the avoid_polygons/exclude_polygons which causes the container to stop. If I send in an empty polygon array to the avoid_polygons array, it causes a segmentation fault:

Screenshot 2023-01-12 110348

Issue occurs when running the following get request:

localhost:8002/route?json={"locations":[{"lat":58.403618,"lon":15.556376},{"lat":58.406674,"lon":15.559801}],"costing":"bus","directionType":"none","avoid_polygons":[[]]}

Beautified json of the request:

{
    "locations": [
        {
            "lat": 58.403618,
            "lon": 15.556376
        },
        {
            "lat": 58.406674,
            "lon": 15.559801
        }
    ],
    "costing": "bus",
    "directionType": "none",
    "avoid_polygons": [
        [

        ]
    ]
}

Expexted behaviour To get a response similar to:

{
    "error_code": 137,
    "error": "Failed to parse polygon",
    "status_code": 400,
    "status": "Bad Request"
} 

Actual behaviour Container crashes and requires a restart


Setup:

I'm using the following docker-compose YAML config to start the container:

version: '3.8'
services:
    valhalla:
        image: gisops/valhalla:latest
        container_name: valhalla_latest
        ports:
            - '8002:8002'
        environment:
            - tile_urls=http://download.geofabrik.de/europe/sweden-latest.osm.pbf
            - build_elevation=False 
            - build_admins=False 
            - build_time_zones=False 
            - force_rebuild=False
nilsnolde commented 1 year ago

Ah, interesting :smile:

Actually I implemented that ages ago and this is the first time I hear about it. It's easily fixed, but not in this project, the fix needs to happen in upstream Valhalla: https://github.com/valhalla/valhalla.

Would you mind opening the issue there? It's enough to refer to this one, which I'll close now.