Changeset 18875 in josm

Timestamp:

2023-10-23T21:06:35+02:00 (7 months ago)

Author:

taylor.smock

Message:

Fix #23238: Some MapCSS functions cause NPEs when part of fixAdd

Location:

trunk

Files:

• src/org/openstreetmap/josm/gui/mappaint/mapcss/Functions.java (modified) (14 diffs)
• test/unit/org/openstreetmap/josm/gui/mappaint/mapcss/FunctionsTest.java (modified) (6 diffs)

trunk/src/org/openstreetmap/josm/gui/mappaint/mapcss/Functions.java

@@ -43 +43 @@
 import org.openstreetmap.josm.tools.RightAndLefthandTraffic;
 import org.openstreetmap.josm.tools.RotationAngle;
+import org.openstreetmap.josm.tools.RotationAngle.WayDirectionRotationAngle;
 import org.openstreetmap.josm.tools.StreamUtils;
 import org.openstreetmap.josm.tools.Territories;
 import org.openstreetmap.josm.tools.Utils;
-import org.openstreetmap.josm.tools.RotationAngle.WayDirectionRotationAngle;
 
 /**
@@ -411 +411 @@
      */
     public static List<String> tag_regex(final Environment env, String keyRegex, String flags) {
+        if (env.osm == null) {
+            return Collections.emptyList();
+        }
         int f = parse_regex_flags(flags);
         Pattern compiled = Pattern.compile(keyRegex, f);
@@ -626 +629 @@
      */
     public static boolean has_tag_key(final Environment env, String key) {
-        return env.osm.hasKey(key);
+        return env.osm != null ? env.osm.hasKey(key) : false;
     }
 
@@ -941 +944 @@
      */
     public static long osm_id(final Environment env) {
-        return env.osm.getUniqueId();
+        return env.osm != null ? env.osm.getUniqueId() : 0;
     }
 
@@ -952 +955 @@
      */
     public static String osm_user_name(final Environment env) {
-        return env.osm.getUser().getName();
+        return env.osm != null ? env.osm.getUser().getName() : null;
     }
 
@@ -963 +966 @@
      */
     public static long osm_user_id(final Environment env) {
-        return env.osm.getUser().getId();
+        return env.osm != null ? env.osm.getUser().getId() : 0;
     }
 
@@ -974 +977 @@
      */
     public static int osm_version(final Environment env) {
-        return env.osm.getVersion();
+        return env.osm != null ? env.osm.getVersion() : 0;
     }
 
@@ -985 +988 @@
      */
     public static int osm_changeset_id(final Environment env) {
-        return env.osm.getChangesetId();
+        return env.osm != null ? env.osm.getChangesetId() : 0;
     }
 
@@ -996 +999 @@
      */
     public static int osm_timestamp(final Environment env) {
-        return env.osm.getRawTimestamp();
+        return env.osm != null ? env.osm.getRawTimestamp() : 0;
     }
 
@@ -1197 +1200 @@
      */
     public static boolean is_right_hand_traffic(Environment env) {
-        return RightAndLefthandTraffic.isRightHandTraffic(center(env));
+        final LatLon center = center(env);
+        if (center != null) {
+            return RightAndLefthandTraffic.isRightHandTraffic(center);
+        }
+        return false;
     }
 
@@ -1261 +1268 @@
      */
     public static int number_of_tags(Environment env) {
-        return env.osm.getNumKeys();
+        return env.osm != null ? env.osm.getNumKeys() : 0;
     }
 
@@ -1271 +1278 @@
      */
     public static Object setting(Environment env, String key) {
-        return env.source.settingValues.get(key);
+        return env.source != null ? env.source.settingValues.get(key) : null;
     }
 
@@ -1281 +1288 @@
      */
     public static LatLon center(Environment env) {
-        return env.osm instanceof Node ? ((Node) env.osm).getCoor() : env.osm.getBBox().getCenter();
+        if (env.osm instanceof ILatLon) {
+            return new LatLon(((ILatLon) env.osm).lat(), ((ILatLon) env.osm).lon());
+        } else if (env.osm != null) {
+            return env.osm.getBBox().getCenter();
+        }
+        return null;
     }
 
@@ -1316 +1328 @@
      */
     public static boolean at(Environment env, double lat, double lon) {
-        return new LatLon(lat, lon).equalsEpsilon(center(env), ILatLon.MAX_SERVER_PRECISION);
+        final ILatLon center = center(env);
+        if (center != null) {
+            return new LatLon(lat, lon).equalsEpsilon(center, ILatLon.MAX_SERVER_PRECISION);
+        }
+        return false;
     }
 

trunk/test/unit/org/openstreetmap/josm/gui/mappaint/mapcss/FunctionsTest.java

@@ -3 +3 @@
 
 import static org.junit.jupiter.api.Assertions.assertAll;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -9 +10 @@
 import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
 import static org.openstreetmap.josm.data.osm.OsmPrimitiveType.NODE;
 
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
-
+import java.util.stream.Stream;
+
+import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
 import org.openstreetmap.josm.TestUtils;
 import org.openstreetmap.josm.data.coor.LatLon;
@@ -30 +38 @@
 import org.openstreetmap.josm.data.preferences.NamedColorProperty;
 import org.openstreetmap.josm.gui.mappaint.Environment;
+import org.openstreetmap.josm.gui.mappaint.MultiCascade;
 import org.openstreetmap.josm.gui.util.GuiHelper;
 import org.openstreetmap.josm.spi.preferences.Config;
@@ -35 +44 @@
 import org.openstreetmap.josm.testutils.annotations.MapPaintStyles;
 import org.openstreetmap.josm.testutils.annotations.Projection;
+import org.openstreetmap.josm.testutils.annotations.Territories;
 
 /**
@@ -62 +72 @@
             return new Environment(osm);
         }
+    }
+
+    private static Method[] FUNCTIONS;
+
+    static Stream<Method> getFunctions() {
+        if (FUNCTIONS == null) {
+            FUNCTIONS = Stream.of(Functions.class.getDeclaredMethods())
+                    .filter(m -> Modifier.isStatic(m.getModifiers()) && Modifier.isPublic(m.getModifiers()))
+                    .toArray(Method[]::new);
+        }
+        return Stream.of(FUNCTIONS);
+    }
+
+    @AfterAll
+    static void tearDown() {
+        FUNCTIONS = null;
     }
 
@@ -231 +257 @@
         assertTrue(Functions.parent_osm_primitives(env, "type2").isEmpty());
     }
+
+    /**
+     * Non-regression test for #23238: NPE when env.osm is null
+     */
+    @ParameterizedTest
+    @MethodSource("getFunctions")
+    @Territories // needed for inside, outside, is_right_hand_traffic
+    void testNonRegression23238(Method function) {
+        if (function.getParameterCount() >= 1 && function.getParameterTypes()[0].isAssignableFrom(Environment.class)
+         && !function.getParameterTypes()[0].equals(Object.class)) {
+            Environment nullOsmEnvironment = new Environment();
+            nullOsmEnvironment.mc = new MultiCascade();
+            Object[] args = new Object[function.getParameterCount()];
+            args[0] = nullOsmEnvironment;
+            for (int i = 1; i < function.getParameterCount(); i++) {
+                final Class<?> type = function.getParameterTypes()[i];
+                if (String.class.isAssignableFrom(type)) {
+                    args[i] = "";
+                } else if (String[].class.isAssignableFrom(type)) {
+                    args[i] = new String[] {"{0}", ""}; // join and tr require at least 2 arguments
+                } else if (Double.class.isAssignableFrom(type) || double.class.isAssignableFrom(type)) {
+                    args[i] = 0d;
+                } else if (Object.class.isAssignableFrom(type)) {
+                    args[i] = new Object[0];
+                } else {
+                    fail(type.getCanonicalName());
+                }
+            }
+            assertDoesNotThrow(() -> function.invoke(null, args));
+        }
+    }
 }