Add Cluster Layer GetMetadata fix with test #6278
Conversation
|
thanks @geographika ! |
| @@ -687,6 +687,11 @@ xmlNodePtr msMetadataGetLayerMetadata(mapObj *map, metadataParamsObj *paramsObj, | |||
| if(strcasecmp(GET_LAYER(map, i)->name, paramsObj->pszLayer) == 0) { | |||
| layer_found = MS_TRUE; | |||
| layer = GET_LAYER(map, i); | |||
| // when checking a layer with clustering msLayerGetExtent does not have access | |||
There was a problem hiding this comment.
I have the feeling this isn't the right fix. That wouldn't fix the crash for someone calling msLayerGetExtent() through the C API or Mapscript, would it ?
There was a problem hiding this comment.
This function is only called using the GetMetadata request (see https://mapserver.org/ogc/layer_metadata.html) - there is no mapscript access currently.
There was a problem hiding this comment.
This function is only called using the
GetMetadatarequest
yes, but isn't the issue fundamentally with msLayerGetExtent() called on a clustered layer ?
There was a problem hiding this comment.
True.. Cluster layers are a special case in msLayerOpen. Then a copy is made of the original layer in msClusterLayerOpen, but it is the source layer vtable which is opened, but then the copied layer doesn't have the original connection - I had a hard time trying to follow exactly what is happening here.
I've not had any cases of msLayerGetExtent calls crashing the server apart from through GetMetadata, but this may be luck/missing logs. The fix in this pull request prevents a server crash, but I agree it would be better to fix the underlying issue.
| @@ -0,0 +1,51 @@ | |||
| # | |||
| # Test a clustered layer with the MSSQL driver plugin | |||
There was a problem hiding this comment.
is the issue specific to using mssql ? Can't the be reproduced with shapefiles ?
There was a problem hiding this comment.
The issue is around a NULL layerinfo->conn in the database driver when trying to get the extent from the database. Shapefiles would be unaffected, but I'd image the PostGIS and Oracle drivers would have the same problem.
Fix for
GetMetadatacalls on a database layer with noEXTENT. See #6277.Adds in a test used to recreate the error (with an additional cluster image test) for a MSSQL layer.
The fix simply removes any clustering in
msMetadataGetLayerMetadata. As clustering won't have an effect on the source metadata, and the function is only called by aGetMetadatathis should have no side-effects.Attempting to modify
msClusterLayerOpento try and copy across connection objects etc. would be an alternative, and more complicated solution, but I'm unaware of any similar issues with other service calls and cluster layers.