[Geoserver-devel] Wondering about one patch I made

Hi Justin,
today I fixed http://jira.codehaus.org/browse/GEOS-1875 and the patch
left me wondering a little.

The thing is, the GetFeature kvp reader did not check at all if the
typeName was good or not... which surprised me more than a little (wcs
tests for example try that and make sure you also set the proper code
and location for the service exception).

So... the patch looked obvious to me, I added a check for the typeName
to be there, and threw a service exception if it was not. Yet, I'm wondering... am I loosing a deep reason why the check was not there?

Cheers
Andrea

PS: patch here:

--- trunk/geoserver/wfs/src/main/java/org/geoserver/wfs/kvp/GetFeatureKvpRequestReader.java 2008-05-05 17:18:42 UTC (rev 8926)
+++ trunk/geoserver/wfs/src/main/java/org/geoserver/wfs/kvp/GetFeatureKvpRequestReader.java 2008-05-05 17:24:27 UTC (rev 8927)
@@ -81,6 +81,13 @@

              for (Iterator itr = typeName.iterator(); itr.hasNext():wink: {
                  QName qName = (QName) itr.next();
+
+ // check the type name is known, otherwise complain
+ if(catalog.getFeatureTypeInfo(qName) == null) {
+ String name = qName.getPrefix() + ":" + qName.getLocalPart();
+ throw new WFSException("Feature type " + name + " unknown", "InvalidParameterValue", "typeName");
+ }
+
                  List l = new ArrayList();
                  l.add(qName);
                  list.add(l);

Interesting... I would have thought this check was already there as well... oh well it cant hurt. If there is some wierd reason for the check not being there wfs 1.1 cite tests should filter it out.. although i cant imagine what it would be.

-Justin

Andrea Aime wrote:

Hi Justin,
today I fixed http://jira.codehaus.org/browse/GEOS-1875 and the patch
left me wondering a little.

The thing is, the GetFeature kvp reader did not check at all if the
typeName was good or not... which surprised me more than a little (wcs
tests for example try that and make sure you also set the proper code
and location for the service exception).

So... the patch looked obvious to me, I added a check for the typeName
to be there, and threw a service exception if it was not. Yet, I'm wondering... am I loosing a deep reason why the check was not there?

Cheers
Andrea

PS: patch here:

--- trunk/geoserver/wfs/src/main/java/org/geoserver/wfs/kvp/GetFeatureKvpRequestReader.java 2008-05-05 17:18:42 UTC (rev 8926)
+++ trunk/geoserver/wfs/src/main/java/org/geoserver/wfs/kvp/GetFeatureKvpRequestReader.java 2008-05-05 17:24:27 UTC (rev 8927)
@@ -81,6 +81,13 @@

              for (Iterator itr = typeName.iterator(); itr.hasNext():wink: {
                  QName qName = (QName) itr.next();
+
+ // check the type name is known, otherwise complain
+ if(catalog.getFeatureTypeInfo(qName) == null) {
+ String name = qName.getPrefix() + ":" + qName.getLocalPart();
+ throw new WFSException("Feature type " + name + " unknown", "InvalidParameterValue", "typeName");
+ }
+
                  List l = new ArrayList();
                  l.add(qName);
                  list.add(l);

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

!DSPAM:4007,481f439940701849620573!

--
Justin Deoliveira
The Open Planning Project
jdeolive@anonymised.com

Justin Deoliveira ha scritto:

Interesting... I would have thought this check was already there as well... oh well it cant hurt. If there is some wierd reason for the check not being there wfs 1.1 cite tests should filter it out.. although i cant imagine what it would be.

Well, eventually you'd have failed with an exception anyways, but
not exactly the one you'd expect (NPE in the case where bbox was
provided along with a wrong type name, for example).

Cheers
Andrea