[Geoserver-devel] [GEOS-10850] Suggested WFS-T OGC exception text improvement

Hi,

We have a use case with GeoServer + MS SQL Server where the WFS module is connected to JDBCStore and WFS-T used for transactional operations.

The DB model consists of two layers; lower layer with a complex data model plus a high level “flat” view layer connected to GeoServer. Triggers in the DB view layer perform data validation, lookups and similar. User-facing application is an OpenLayers based web map and when the user is creating, editing or deleting features we use WFS-T between OpenLayers and GeoServer.

An issue with the current GeoServer WFS module is that the root cause on any storage exception is lost in the OGC exception report returned by WFS-T service. E.g. if SQL error is raised with a message regarding failed validation, missing lookup or similar.

I created a JIRA issue for a suggested improvement and have started work on a PR in a Git fork.

See https://osgeo-org.atlassian.net/browse/GEOS-10850 and https://github.com/sweco-semara/geoserver/commit/6c5139fe66091a1822bce643673f9018245c8c62

This seems to work fine, but before creating a PR I need developer’s feedback on:

···
  1. Is this change welcome in GeoServer? We see an improvement for our use-case and there are no existing tests that fail.
  2. If it’s a welcome change, could a GS developer give a hint as to where to move current code duplication for message creation (StringBuilder handling – see “TODO” in the commit above). I have seen both static utils/static helper classes and embedded logic in e.g. exception classes in GeoServer codebase, so I am unsure what the current coding convention would be?

Thanks for your time.

Best regards,

Martin

Martin:

I personally welcome such a change; but must advise you that every time we include real information from the database in an OGC exception text we get a security vulnerability report. Folks are especially careful with database error codes appearing anywhere in output (even exception output).

As long as your improvement respects the “include stack trace” setting you should be fine.

I recognize the duplication but do not know which approach is best for consistency.


Jody Garnett

On Fri, Feb 3, 2023 at 6:17 PM Kalén, Martin via Geoserver-devel <geoserver-devel@lists.sourceforge.net> wrote:

Hi,

We have a use case with GeoServer + MS SQL Server where the WFS module is connected to JDBCStore and WFS-T used for transactional operations.

The DB model consists of two layers; lower layer with a complex data model plus a high level “flat” view layer connected to GeoServer. Triggers in the DB view layer perform data validation, lookups and similar. User-facing application is an OpenLayers based web map and when the user is creating, editing or deleting features we use WFS-T between OpenLayers and GeoServer.

An issue with the current GeoServer WFS module is that the root cause on any storage exception is lost in the OGC exception report returned by WFS-T service. E.g. if SQL error is raised with a message regarding failed validation, missing lookup or similar.

I created a JIRA issue for a suggested improvement and have started work on a PR in a Git fork.

See https://osgeo-org.atlassian.net/browse/GEOS-10850 and https://github.com/sweco-semara/geoserver/commit/6c5139fe66091a1822bce643673f9018245c8c62

This seems to work fine, but before creating a PR I need developer’s feedback on:

  1. Is this change welcome in GeoServer? We see an improvement for our use-case and there are no existing tests that fail.
  2. If it’s a welcome change, could a GS developer give a hint as to where to move current code duplication for message creation (StringBuilder handling – see “TODO” in the commit above). I have seen both static utils/static helper classes and embedded logic in e.g. exception classes in GeoServer codebase, so I am unsure what the current coding convention would be?

Thanks for your time.

Best regards,

Martin


Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel