Changeset 18195 in josm

Timestamp:

2021-09-07T23:46:34+02:00 (3 years ago)

Author:

Don-vip

Message:

fix #21300 - NPE in search compiler (patch by taylor.smock)

Location:

trunk

Files:

• src/org/openstreetmap/josm/data/osm/search/SearchCompiler.java (modified) (5 diffs)
• test/unit/org/openstreetmap/josm/data/osm/search/SearchCompilerTest.java (modified) (4 diffs)

trunk/src/org/openstreetmap/josm/data/osm/search/SearchCompiler.java

@@ -291 +291 @@
             return match(object);
         }
+
+        /**
+         * Check if this is a valid match object
+         * @return {@code this}, for easy chaining
+         * @throws SearchParseError If the match is not valid
+         */
+        public Match validate() throws SearchParseError {
+            // Default to no-op
+            return this;
+        }
     }
 
@@ -861 +871 @@
                 return false;
             return true;
+        }
+
+        @Override
+        public Match validate() throws SearchParseError {
+            if (this.referenceValue == null) {
+                final String referenceType;
+                if (this.compareMode == +1) {
+                    referenceType = ">";
+                } else if (this.compareMode == -1) {
+                    referenceType = "<";
+                } else {
+                    referenceType = "<unknown>";
+                }
+                throw new SearchParseError(tr("Reference value for ''{0}'' expected", referenceType));
+            }
+            return this;
         }
     }
@@ -2109 +2135 @@
             String key = tokenizer.getText();
             if (tokenizer.readIfEqual(Token.EQUALS)) {
-                return new ExactKeyValue(regexSearch, caseSensitive, key, tokenizer.readTextOrNumber());
+                return new ExactKeyValue(regexSearch, caseSensitive, key, tokenizer.readTextOrNumber()).validate();
             } else if (tokenizer.readIfEqual(Token.TILDE)) {
-                return new ExactKeyValue(true, caseSensitive, key, tokenizer.readTextOrNumber());
+                return new ExactKeyValue(true, caseSensitive, key, tokenizer.readTextOrNumber()).validate();
             } else if (tokenizer.readIfEqual(Token.LESS_THAN)) {
-                return new ValueComparison(key, tokenizer.readTextOrNumber(), -1);
+                return new ValueComparison(key, tokenizer.readTextOrNumber(), -1).validate();
             } else if (tokenizer.readIfEqual(Token.GREATER_THAN)) {
-                return new ValueComparison(key, tokenizer.readTextOrNumber(), +1);
+                return new ValueComparison(key, tokenizer.readTextOrNumber(), +1).validate();
             } else if (tokenizer.readIfEqual(Token.COLON)) {
                 // see if we have a Match that takes a data parameter
@@ -2124 +2150 @@
                 UnaryMatchFactory unaryFactory = unaryMatchFactoryMap.get(key);
                 if (unaryFactory != null)
-                    return unaryFactory.get(key, parseFactor(), tokenizer);
+                    return unaryFactory.get(key, parseFactor(), tokenizer).validate();
 
                 // key:value form where value is a string (may be OSM key search)
                 final String value = tokenizer.readTextOrNumber();
-                return new KeyValue(key, value != null ? value : "", regexSearch, caseSensitive);
+                return new KeyValue(key, value != null ? value : "", regexSearch, caseSensitive).validate();
             } else if (tokenizer.readIfEqual(Token.QUESTION_MARK))
                 return new BooleanMatch(key, false);
@@ -2134 +2160 @@
                 SimpleMatchFactory factory = simpleMatchFactoryMap.get(key);
                 if (factory != null)
-                    return factory.get(key, caseSensitive, regexSearch, null);
+                    return factory.get(key, caseSensitive, regexSearch, null).validate();
 
                 UnaryMatchFactory unaryFactory = unaryMatchFactoryMap.get(key);
                 if (unaryFactory != null)
-                    return unaryFactory.get(key, parseFactor(), null);
+                    return unaryFactory.get(key, parseFactor(), null).validate();
 
                 // match string in any key or value
-                return new Any(key, regexSearch, caseSensitive);
+                return new Any(key, regexSearch, caseSensitive).validate();
             }
         } else

trunk/test/unit/org/openstreetmap/josm/data/osm/search/SearchCompilerTest.java

@@ -19 +19 @@
 import java.util.Set;
 
+import org.junit.Assert;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 import org.openstreetmap.josm.TestUtils;
 import org.openstreetmap.josm.data.coor.LatLon;
@@ -45 +50 @@
 import nl.jqno.equalsverifier.EqualsVerifier;
 import nl.jqno.equalsverifier.Warning;
-import org.junit.Assert;
-import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.Timeout;
 
 /**
@@ -145 +147 @@
      */
     @Test
-    void testRegexpCaseSensitive() throws Exception {
+    void testRegexpCaseSensitive() throws SearchParseError {
         SearchSetting searchSetting = new SearchSetting();
         searchSetting.regexSearch = true;
@@ -829 +831 @@
         sc.match(sc.n2, false);
     }
+
+    /**
+     * Non-regression test for JOSM #21300
+     * @param searchString search string to test
+     */
+    @ParameterizedTest
+    @ValueSource(strings = {"maxweight<" /* #21300 */, "maxweight>"})
+    void testNonRegression21300(final String searchString) {
+        assertThrows(SearchParseError.class, () -> SearchCompiler.compile(searchString));
+    }
 }