search

placemark / check-geojson

a checker for the geojson format. goes beyond a schema, checking semantics and producing character-level warnings.
http://check-geojson.docs.placemark.io/
MIT License
72 stars 2 forks source link

Does not throw with some invalid geojson #30

Open Portur opened 1 year ago

Portur commented 1 year ago

Hi, here are tests that I expected to throw but don't. Sorry for the long post. It seems sending the value as object does not throw, where sending the same object as stringdoes throw. I have added additional invalid types and which ones fail.

The objects I have tested that cause the function to not throw are:

const { check: geojsonTester } = require("@placemarkio/check-geojson");

// https://github.com/chrieke/geojson-invalid-geometry/blob/main/examples_geojson/invalid/polygon_unclosed_polygon.geojson?short_path=2a32854
const invalidGeojsonGeometryPolygonUnclosed = {
  type: "Polygon",
  coordinates: [
    [
      [13.376753, 52.515641],
      [13.37696, 52.515011],
      [13.378033, 52.514998],
      [13.378049, 52.516176],
    ],
  ],
};

// https://github.com/chrieke/geojson-invalid-geometry/blob/main/examples_geojson/invalid/polygon_has_duplicate_nodes.geojson?short_path=487c8b7
const invalidGeojsonGeometryPolygonDuplicatesNodes = {
  type: "Polygon",
  coordinates: [
    [
      [13.378261, 52.513389],
      [13.377365, 52.51446],
      [13.377365, 52.51446],
      [13.376762, 52.51337],
      [13.378261, 52.513389],
    ],
  ],
};

// https://github.com/chrieke/geojson-invalid-geometry/blob/main/examples_geojson/invalid/polygon_has_less_than_three_unique_nodes.geojson?short_path=e1e556d
const invalidGeojsonGeometryPolygonTwoNodes = {
  type: "Polygon",
  coordinates: [
    [
      [13.377016, 52.512418],
      [13.378182, 52.51285],
      [13.377016, 52.512418],
    ],
  ],
};

// https://github.com/chrieke/geojson-invalid-geometry/blob/main/examples_geojson/invalid/polygon_interior_ring_not_clockwise_winding_order.geojson?short_path=1df04b0
const invalidGeojsonGeometryPolygonWithHolesCounterClockwise = {
  type: "Polygon",
  coordinates: [
    [
      [13.379307, 52.512661],
      [13.381102, 52.512687],
      [13.380973, 52.514197],
      [13.379281, 52.514133],
      [13.379307, 52.512661],
    ],
    [
      [13.380443, 52.513849],
      [13.379901, 52.513849],
      [13.379669, 52.51372],
      [13.379591, 52.513384],
      [13.379669, 52.512945],
      [13.380185, 52.512855],
      [13.380676, 52.51301],
      [13.380559, 52.513294],
      [13.379979, 52.513603],
      [13.380443, 52.513849],
    ],
  ],
};

// https://github.com/chrieke/geojson-invalid-geometry/blob/main/examples_geojson/invalid/polygon_exterior_ring_not_counterclockwise_winding_order.geojson?short_path=f7e7cf0
const invalidGeojsonGeometryPolygonWithoutHolesCounterClockwise = {
  type: "Polygon",
  coordinates: [
    [
      [13.379263, 52.516012],
      [13.380039, 52.516012],
      [13.380557, 52.515852],
      [13.380756, 52.515275],
      [13.380597, 52.514996],
      [13.380139, 52.514718],
      [13.379601, 52.514757],
      [13.379422, 52.514937],
      [13.379522, 52.515534],
      [13.379701, 52.515833],
      [13.379263, 52.516012],
    ],
  ],
};

// https://github.com/chrieke/geojson-invalid-geometry/blob/main/examples_geojson/invalid/polygon_inner_and_exterior_ring_cross.geojson?short_path=dae4bfa
const invalidGeojsonGeometryPolygonSelfIntersect = {
  type: "Polygon",
  coordinates: [
    [
      [13.382288, 52.515426],
      [13.382096, 52.514797],
      [13.383424, 52.51464],
      [13.383529, 52.515496],
      [13.382288, 52.515426],
    ],
    [
      [13.382603, 52.514954],
      [13.38262, 52.516255],
      [13.383144, 52.515321],
      [13.383127, 52.514867],
      [13.382603, 52.514954],
    ],
  ],
};

// https://github.com/chrieke/geojson-invalid-geometry/blob/main/examples_geojson/invalid/linestring_zero_length.geojson?short_path=14d173c
const invalidGeojsonGeometryLineZeroLength = {
  type: "LineString",
  coordinates: [
    [13.39265, 52.515046],
    [13.39265, 52.515046],
  ],
};

// https://github.com/chrieke/geojson-invalid-geometry/blob/main/examples_geojson/invalid/incorrect_geometry_data_type.geojson?short_path=01db740
const invalidGeojsonGeometryType = {
  type: "LineString",
  coordinates: [
    [
      [13.383092, 52.510235],
      [13.383092, 52.509704],
      [13.38458, 52.509704],
      [13.38458, 52.510235],
      [13.383092, 52.510235],
    ],
  ],
};

try {
  geojsonTester(JSON.stringify(invalidGeojsonGeometryLineZeroLength));
  console.log("string invalidGeojsonGeometryLineZeroLength -fail");
} catch (e) {
  console.log("string invalidGeojsonGeometryLineZeroLength -pass");
}

try {
  geojsonTester(invalidGeojsonGeometryLineZeroLength);
  console.log("object invalidGeojsonGeometryLineZeroLength -fail");
} catch (e) {
  console.log("object invalidGeojsonGeometryLineZeroLength -pass");
}

try {
  geojsonTester(JSON.stringify(invalidGeojsonGeometryPolygonSelfIntersect));
  console.log("string invalidGeojsonGeometryPolygonSelfIntersect -fail");
} catch (e) {
  console.log("string invalidGeojsonGeometryPolygonSelfIntersect -pass");
}

try {
  geojsonTester(invalidGeojsonGeometryPolygonSelfIntersect);
  console.log("object invalidGeojsonGeometryPolygonSelfIntersect -fail");
} catch (e) {
  console.log("object invalidGeojsonGeometryPolygonSelfIntersect -pass");
}

try {
  geojsonTester(JSON.stringify(invalidGeojsonGeometryPolygonUnclosed));
  console.log("string invalidGeojsonGeometryPolygonUnclosed -fail");
} catch (e) {
  console.log("string invalidGeojsonGeometryPolygonUnclosed -pass");
}

try {
  geojsonTester(invalidGeojsonGeometryPolygonUnclosed);
  console.log("object invalidGeojsonGeometryPolygonUnclosed -fail");
} catch (e) {
  console.log("object invalidGeojsonGeometryPolygonUnclosed -pass");
}

string invalidGeojsonGeometryLineZeroLength -fail
object invalidGeojsonGeometryLineZeroLength -pass
string invalidGeojsonGeometryPolygonSelfIntersect -fail
object invalidGeojsonGeometryPolygonSelfIntersect -pass
string invalidGeojsonGeometryPolygonUnclosed -pass
object invalidGeojsonGeometryPolygonUnclosed -pass
tmcw commented 1 year ago

The check method takes a string as an argument - passing it an object or any other type is undefined behavior. If there's a bug with check + a string argument, happy to look at it, but the documentation and TypeScript types dictate that it is only to be used with string arguments.

Portur commented 1 year ago

Ok thanks I missed that.

There is however still the issue where these invalid geoms arent being seen as invalid

const { check: geojsonTester } = require("@placemarkio/check-geojson");

// https://github.com/chrieke/geojson-invalid-geometry/blob/main/examples_geojson/invalid/polygon_unclosed_polygon.geojson?short_path=2a32854
const invalidGeojsonGeometryPolygonUnclosed = {
  type: "Polygon",
  coordinates: [
    [
      [13.376753, 52.515641],
      [13.37696, 52.515011],
      [13.378033, 52.514998],
      [13.378049, 52.516176],
    ],
  ],
};
// https://github.com/chrieke/geojson-invalid-geometry/blob/main/examples_geojson/invalid/polygon_has_duplicate_nodes.geojson?short_path=487c8b7
const invalidGeojsonGeometryPolygonDuplicatesNodes = {
  type: "Polygon",
  coordinates: [
    [
      [13.378261, 52.513389],
      [13.377365, 52.51446],
      [13.377365, 52.51446],
      [13.376762, 52.51337],
      [13.378261, 52.513389],
    ],
  ],
};
// https://github.com/chrieke/geojson-invalid-geometry/blob/main/examples_geojson/invalid/polygon_has_less_than_three_unique_nodes.geojson?short_path=e1e556d
const invalidGeojsonGeometryPolygonTwoNodes = {
  type: "Polygon",
  coordinates: [
    [
      [13.377016, 52.512418],
      [13.378182, 52.51285],
      [13.377016, 52.512418],
    ],
  ],
};
// https://github.com/chrieke/geojson-invalid-geometry/blob/main/examples_geojson/invalid/polygon_interior_ring_not_clockwise_winding_order.geojson?short_path=1df04b0
const invalidGeojsonGeometryPolygonWithHolesCounterClockwise = {
  type: "Polygon",
  coordinates: [
    [
      [13.379307, 52.512661],
      [13.381102, 52.512687],
      [13.380973, 52.514197],
      [13.379281, 52.514133],
      [13.379307, 52.512661],
    ],
    [
      [13.380443, 52.513849],
      [13.379901, 52.513849],
      [13.379669, 52.51372],
      [13.379591, 52.513384],
      [13.379669, 52.512945],
      [13.380185, 52.512855],
      [13.380676, 52.51301],
      [13.380559, 52.513294],
      [13.379979, 52.513603],
      [13.380443, 52.513849],
    ],
  ],
};
// https://github.com/chrieke/geojson-invalid-geometry/blob/main/examples_geojson/invalid/polygon_exterior_ring_not_counterclockwise_winding_order.geojson?short_path=f7e7cf0
const invalidGeojsonGeometryPolygonWithoutHolesCounterClockwise = {
  type: "Polygon",
  coordinates: [
    [
      [13.379263, 52.516012],
      [13.380039, 52.516012],
      [13.380557, 52.515852],
      [13.380756, 52.515275],
      [13.380597, 52.514996],
      [13.380139, 52.514718],
      [13.379601, 52.514757],
      [13.379422, 52.514937],
      [13.379522, 52.515534],
      [13.379701, 52.515833],
      [13.379263, 52.516012],
    ],
  ],
};
// https://github.com/chrieke/geojson-invalid-geometry/blob/main/examples_geojson/invalid/polygon_inner_and_exterior_ring_cross.geojson?short_path=dae4bfa
const invalidGeojsonGeometryPolygonSelfIntersect = {
  type: "Polygon",
  coordinates: [
    [
      [13.382288, 52.515426],
      [13.382096, 52.514797],
      [13.383424, 52.51464],
      [13.383529, 52.515496],
      [13.382288, 52.515426],
    ],
    [
      [13.382603, 52.514954],
      [13.38262, 52.516255],
      [13.383144, 52.515321],
      [13.383127, 52.514867],
      [13.382603, 52.514954],
    ],
  ],
};
// https://github.com/chrieke/geojson-invalid-geometry/blob/main/examples_geojson/invalid/linestring_zero_length.geojson?short_path=14d173c
const invalidGeojsonGeometryLineZeroLength = {
  type: "LineString",
  coordinates: [
    [13.39265, 52.515046],
    [13.39265, 52.515046],
  ],
};
// https://github.com/chrieke/geojson-invalid-geometry/blob/main/examples_geojson/invalid/incorrect_geometry_data_type.geojson?short_path=01db740
const invalidGeojsonGeometryType = {
  type: "LineString",
  coordinates: [
    [
      [13.383092, 52.510235],
      [13.383092, 52.509704],
      [13.38458, 52.509704],
      [13.38458, 52.510235],
      [13.383092, 52.510235],
    ],
  ],
};

try {
  geojsonTester(JSON.stringify(invalidGeojsonGeometryPolygonUnclosed));
  console.log("fail - invalidGeojsonGeometryPolygonUnclosed");
} catch (e) {
  console.log("pass - invalidGeojsonGeometryPolygonUnclosed");
}

try {
  geojsonTester(JSON.stringify(invalidGeojsonGeometryPolygonDuplicatesNodes));
  console.log("fail - invalidGeojsonGeometryPolygonDuplicatesNodes");
} catch (e) {
  console.log("pass - invalidGeojsonGeometryPolygonDuplicatesNodes");
}

try {
  geojsonTester(JSON.stringify(invalidGeojsonGeometryPolygonTwoNodes));
  console.log("fail - invalidGeojsonGeometryPolygonTwoNodes");
} catch (e) {
  console.log("pass - invalidGeojsonGeometryPolygonTwoNodes");
}

try {
  geojsonTester(JSON.stringify(invalidGeojsonGeometryPolygonWithHolesCounterClockwise));
  console.log("fail - invalidGeojsonGeometryPolygonWithHolesCounterClockwise");
} catch (e) {
  console.log("pass - invalidGeojsonGeometryPolygonWithHolesCounterClockwise");
}

try {
  geojsonTester(JSON.stringify(invalidGeojsonGeometryPolygonWithoutHolesCounterClockwise));
  console.log("fail - invalidGeojsonGeometryPolygonWithoutHolesCounterClockwise");
} catch (e) {
  console.log("pass - invalidGeojsonGeometryPolygonWithoutHolesCounterClockwise");
}

try {
  geojsonTester(JSON.stringify(invalidGeojsonGeometryPolygonSelfIntersect));
  console.log("fail - invalidGeojsonGeometryPolygonSelfIntersect");
} catch (e) {
  console.log("pass - invalidGeojsonGeometryPolygonSelfIntersect");
}

try {
  geojsonTester(JSON.stringify(invalidGeojsonGeometryLineZeroLength));
  console.log("fail - invalidGeojsonGeometryLineZeroLength");
} catch (e) {
  console.log("pass - invalidGeojsonGeometryLineZeroLength");
}

try {
  geojsonTester(JSON.stringify(invalidGeojsonGeometryType));
  console.log("fail - invalidGeojsonGeometryType");
} catch (e) {
  console.log("pass - invalidGeojsonGeometryType");
}

pass - invalidGeojsonGeometryPolygonUnclosed
fail - invalidGeojsonGeometryPolygonDuplicatesNodes
pass - invalidGeojsonGeometryPolygonTwoNodes
fail - invalidGeojsonGeometryPolygonWithHolesCounterClockwise
fail - invalidGeojsonGeometryPolygonWithoutHolesCounterClockwise
fail - invalidGeojsonGeometryPolygonSelfIntersect
fail - invalidGeojsonGeometryLineZeroLength
pass - invalidGeojsonGeometryType
Portur commented 1 year ago

@tmcw I just want to raise awareness of the above - where even using the values as string will still fail to check the invalid geometries above

tmcw commented 1 year ago

Thanks, I think we should look at the invalid LineString & the three-node & unclosed Polygon test cases - whoops, just realized that those ones are passing.

The others are a little iffy for inclusion in this library. For the Right-hand-rule, we're following the current spec guidance:

Note: the [GJ2008] specification did not discuss linear ring winding order. For backwards compatibility, parsers SHOULD NOT reject Polygons that do not follow the right-hand rule.

Detecting & throwing on invalid geometries, like self-intersections, is pretty out-of-scope for this module as-is (though if someone wanted to contribute a robust implementation, I wouldn't be opposed.) Basically, we're checking structural validity and things defined in the GeoJSON spec. The GeoJSON spec doesn't even mention self-intersections - this is deferred to the WFS simple features spec. Checking things like self-intersections requires robust implementations of a bunch of geometry algorithms that can be much more performance-intensive than anything in this module, and also should probably be in a module that's more format-agnostic.

Portur commented 1 year ago

I'll do some investigating and come back. I don't think I have the knowledge or experience to contribute in any way but I'll see what I can find on how others have solved performant checks for self intersection.