Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid GML srsDimension on GmlGeoX v1 #102

Closed
carlospzurita opened this issue Dec 4, 2020 · 4 comments
Closed

Invalid GML srsDimension on GmlGeoX v1 #102

carlospzurita opened this issue Dec 4, 2020 · 4 comments
Labels
EIP-approved EIP approved by the Steering Group Impl. EIP has been implemented and is ready for the next release. Doc might be incomplete (temp. label)
Milestone

Comments

@carlospzurita
Copy link

carlospzurita commented Dec 4, 2020

Background and Motivation:

It has been detected that during the parsing of some GML files errors occur in the etf-bsxtd library.
These are files in which the number of dimensions of the geometry has been declared using the srsDimension attribute.

The error occurs when the value indicated in that attribute does not match the number of dimensions associated with the SRS declared in the geometry file.

The error seems to occur only when the srsDimension attribute is declared in the same tag where the srsName attribute is indicated. However, it disappears when srsDimension is moved to the gml:posList tag that contains the list of coordinates.

This is a memory leak that only manifests itself when the geometry is large enough, although it is linked to parsing errors even when the testdriver execution is completed.

Each CRS has implicitly associated the number of dimensions of the geometry expressed in that CRS.
Thus, for example, declaring srsName="http://www.opengis.net/def/crs/EPSG/0/4258" establishes that the geometry must have 2 dimensions.
When the srsDimension="3" attribute is declared in the same tag, the value of srsDimension is ignored, and the coordinates of the file are parsed incorrectly.

In that case, errors like this occur:

The dataset has 1 feature(s) with errors for this assertion.
XML document 'PAUWWTD2016_PL-reducido.gml', ManagementRestrictionOrRegulationZone 'PAUWWTD_PLUWWTD': The feature geometry is not a valid GML geometry. Error detected: Invalid polygon. Ring is not closed within element ManagementRestrictionOrRegulationZone, (gml:id: PAUWWTD_PLUWWTD with coordinates: LINESTRING (54.852199 18.300598,0.000000 54.852256,18.299551...). Starting point (54.8521992966 18.300598287) is not the same as end point (18.300598287 0.000).

That is, a two-dimensional geometry is interpreted from a list of three-dimensional coordinates.

This produces a mismatch in all coordinates, a change in the number of points in the geometry, prevents the first and last point from coinciding in closed geometries, etc.
Also, if the number of coordinates is not multiple of 6 in this example (the least common multiple of the SRS dimensions and those declared in the file), not all the points can be correctly constructed and memory errors can occur during the parse.

This is not an error in the validation of the GML file, but the parsing of the file prior to validation.

The deegree library recognizes and parses correctly the srsDimension attribute in the gml:posList tag, but not in gml:Surface, where the srsName attribute is indicated. However, there are GML files that indicate the number of dimensions precisely in that tag.

This same problem with certain GML files occurs in other tools:
https://trac.osgeo.org/gdal/ticket/5606
http://osgeo-org.1560.x6.nabble.com/Adding-srsDimension-at-the-posList-level-too-td5087644.html

Proposed change

A pre-validation of GML files is proposed to reject those that indicate a number of dimensions in the geometry other than the number of dimensions corresponding to the indicated SRS.
This pre-validation would be done before the validation with deegree.

The following changes are proposed:

  • Add a new class for GML file pre-validation. This class checks that the attributes are in the corresponding tags and that the number of dimensions of the geometry corresponds to the declared SRS. This pre-validation is done from the GML, not from the already built geometry, so it would take advantage of the GmlStreamReader already built in the current code.
  • Add the call to the new pre-validation in the method validate(ValidatorContext validatorContext, Element element) of the GeometryElementHandler class, after the construction of the GmlStreamReader, and before the call to readGeometry().

If the GML file does not pass the pre-validation, corresponding exceptions are added in the GeometryElementHandler to generate an additional message for the element indicating that it does not comply with the correct GML format.

It is considered that the same changes of GeometryElementHandler should be reproduced in the SecondaryGeometryElementValidationHandler class.

Alternatives

In addition to the proposed solution described in the previous section, these other alternatives have been considered:

  1. Modify deegree to accept the srsDimension attribute in the upper levels of the hierarchy, before the specification of the list of coordinates in the gml:posList tag.
  2. Modify etf-gmlgeox to alter the input GML stream by adding the srsDimension attribute inherited from the upper levels of the hierarchy to its lower levels (including the gml:posList tag).

Funding

N/A

Additional information

N/A

@MarcoMinghini MarcoMinghini added the EIP Improvement Proposal. Put up for discussion. label Dec 16, 2020
@cportele cportele added EIP-approved EIP approved by the Steering Group and removed EIP Improvement Proposal. Put up for discussion. labels Dec 17, 2020
@cportele
Copy link
Contributor

SG27: The EIP is approved. Next step: Create a PR.

@jonherrmann jonherrmann added the ? Unknown status or information missing in the EIP (temp. label) label May 23, 2022
@jonherrmann
Copy link
Contributor

@jonherrmann jonherrmann added Impl. EIP has been implemented and is ready for the next release. Doc might be incomplete (temp. label) and removed ? Unknown status or information missing in the EIP (temp. label) labels May 24, 2022
@jonherrmann
Copy link
Contributor

Implemented in Version 2.1.0

@jonherrmann
Copy link
Contributor

Implemented in Version 2.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
EIP-approved EIP approved by the Steering Group Impl. EIP has been implemented and is ready for the next release. Doc might be incomplete (temp. label)
Projects
None yet
Development

No branches or pull requests

4 participants