search

tidwall / geojson

GeoJSON for Go. Used by Tile38
MIT License
130 stars 28 forks source link

Check to prevent panic #26

Open prathik opened 9 months ago

prathik commented 9 months ago

@tidwall I have added a check to prevent panics in the library, if I remove this check and you run the test, you can see the panic.

Do you have any recommendation on how we go about fixing this issue? If there are one set of coordinates with z coordinates and other without, they way the library handles it is to allocate the z coordinate values to the values that come first, instead of where the values actually should go.

I see few approaches:

  1. Arrange the coordinates in such a way that those have a z value would go to the right place, however this might see an issue with linear ring polygons that have some z values and others don't

  2. Assume a default z coordinate value if some have missing z values

  3. Refactor the way we store the ex.values array so that it also knows which particular entry should get the z coordinate.

prathik commented 9 months ago

@tidwall Just for context, we had inserted a polygon of this format in tile38 and on reading it - the panic caused the whole tile38 server to crash, so might be a good idea to fix this.

tidwall commented 9 months ago

Hi, I recommend filing an issue in Tile38 that includes the commands that caused the server to crash. That would make it easier for me to diagnose and fix.

prathik commented 9 months ago

@tidwall I've raised it here - https://github.com/tidwall/tile38/issues/714