Skip to content

Commit

Permalink
[GEOS-11720] AttributeTypeInfoImpl doesn't quote names properly
Browse files Browse the repository at this point in the history
At least two issues found here:
1. Simply being able to parse a string as ECQL doesn't tell us if the
resulting ECQL would do what we want - what we need to know, before
deciding to apply quotes, is if the ECQL parser would currently parse
this field name as a PropertyName (what we want) or something else.
2. Not all ECQL parsing failures are CQLException - there is also
EmptyStackException and there may be more.

I found this issue by trying to create a feature type with a field
called "POINT". This failed with the following stack trace:

java.util.EmptyStackException
    at java.base/java.util.Stack.peek(Unknown Source)
    at java.base/java.util.Stack.pop(Unknown Source)
    -- lots of ECQL stack trace removed for brevity --
    at org.geotools.filter.text.ecql.ECQL.toExpression(ECQL.java:125)
    at org.geoserver.catalog.impl.AttributeTypeInfoImpl.getSource(AttributeTypeInfoImpl.java:147)
  • Loading branch information
olsen232 authored and aaime committed Feb 24, 2025
1 parent fc0bcb0 commit d08472b
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
import org.geoserver.catalog.MetadataMap;
import org.geoserver.util.InternationalStringUtils;
import org.geotools.api.feature.type.AttributeDescriptor;
import org.geotools.api.filter.expression.PropertyName;
import org.geotools.api.util.InternationalString;
import org.geotools.filter.text.cql2.CQLException;
import org.geotools.filter.text.ecql.ECQL;
import org.geotools.util.GrowableInternationalString;

Expand Down Expand Up @@ -142,20 +142,25 @@ public String getRawSource() {
@Override
public String getSource() {
if (source == null && name != null) {
try {
// is it usable as is?
ECQL.toExpression(name);
// even if parseable, dots should be escaped
if (name.contains(".")) return "\"" + name + "\"";
return name;
} catch (CQLException e) {
// quoting to avoid reserved keyword issues
return "\"" + name + "\"";
}
return needsQuoting(name) ? ("\"" + name + "\"") : name;
}
return source;
}

private static boolean needsQuoting(String name) {
if (name.contains(".")) {
return true;
}
try {
if (!(ECQL.toExpression(name) instanceof PropertyName)) {
return true;
}
} catch (Exception e) {
return true;
}
return false;
}

@Override
public void setSource(String source) {
this.source = source;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/* (c) 2025 Open Source Geospatial Foundation - all rights reserved
* This code is licensed under the GPL 2.0 license, available at the root
* application directory.
*/
package org.geoserver.catalog.impl;

import static org.junit.Assert.assertEquals;

import org.junit.Test;

public class AttributeTypeInfoImplTest {
@Test
public void testQuoting() {
assertNotQuoted("foo");
assertNotQuoted("foo123");
assertNotQuoted("FOO");
assertNotQuoted("FOO123");
assertNotQuoted("foo_bar");
assertNotQuoted("Foo_Bar");

assertQuoted("123foo");
assertQuoted("foo.bar");
assertQuoted("point");
assertQuoted("POINT");
assertQuoted("BBOX");
assertQuoted("intersects");
}

private void assertNotQuoted(String name) {
AttributeTypeInfoImpl a = new AttributeTypeInfoImpl();
a.setName(name);
assertEquals(name, a.getSource());
}

private void assertQuoted(String name) {
AttributeTypeInfoImpl a = new AttributeTypeInfoImpl();
a.setName(name);
assertEquals("\"" + name + "\"", a.getSource());
}
}

0 comments on commit d08472b

Please sign in to comment.