Skip to content

Commit

Permalink
Fix #23238: Some MapCSS functions cause NPEs when part of fixAdd
Browse files Browse the repository at this point in the history 
git-svn-id: https://josm.openstreetmap.de/svn/trunk@18875 0c6e7542-c601-0410-84e7-c038aed88b3b
  • Loading branch information
taylor.smock committed Oct 23, 2023
1 parent f7a64bd commit 8203bfd
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 13 deletions.
42 changes: 29 additions & 13 deletions src/org/openstreetmap/josm/gui/mappaint/mapcss/Functions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@
import org.openstreetmap.josm.tools.Logging;
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;

/**
* List of functions that can be used in MapCSS expressions.
Expand Down Expand Up @@ -410,6 +410,9 @@ public static List<String> tag_regex(final Environment env, String keyRegex) {
* @since 15315
*/
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);
return env.osm.getKeys().entrySet().stream()
Expand Down Expand Up @@ -625,7 +628,7 @@ public static double gpx_distance(final Environment env) {
* @return {@code true} if the object has a tag with the given key, {@code false} otherwise
*/
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;
}

/**
Expand Down Expand Up @@ -940,7 +943,7 @@ public static List<String> regexp_match(String pattern, String target) {
* @see IPrimitive#getUniqueId()
*/
public static long osm_id(final Environment env) {
return env.osm.getUniqueId();
return env.osm != null ? env.osm.getUniqueId() : 0;
}

/**
Expand All @@ -951,7 +954,7 @@ public static long osm_id(final Environment env) {
* @since 15246
*/
public static String osm_user_name(final Environment env) {
return env.osm.getUser().getName();
return env.osm != null ? env.osm.getUser().getName() : null;
}

/**
Expand All @@ -962,7 +965,7 @@ public static String osm_user_name(final Environment env) {
* @since 15246
*/
public static long osm_user_id(final Environment env) {
return env.osm.getUser().getId();
return env.osm != null ? env.osm.getUser().getId() : 0;
}

/**
Expand All @@ -973,7 +976,7 @@ public static long osm_user_id(final Environment env) {
* @since 15246
*/
public static int osm_version(final Environment env) {
return env.osm.getVersion();
return env.osm != null ? env.osm.getVersion() : 0;
}

/**
Expand All @@ -984,7 +987,7 @@ public static int osm_version(final Environment env) {
* @since 15246
*/
public static int osm_changeset_id(final Environment env) {
return env.osm.getChangesetId();
return env.osm != null ? env.osm.getChangesetId() : 0;
}

/**
Expand All @@ -995,7 +998,7 @@ public static int osm_changeset_id(final Environment env) {
* @since 15246
*/
public static int osm_timestamp(final Environment env) {
return env.osm.getRawTimestamp();
return env.osm != null ? env.osm.getRawTimestamp() : 0;
}

/**
Expand Down Expand Up @@ -1196,7 +1199,11 @@ public static long CRC32_checksum(String s) {
* @since 7193
*/
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;
}

/**
Expand Down Expand Up @@ -1260,7 +1267,7 @@ public static Object println(Object o) {
* @return number of tags
*/
public static int number_of_tags(Environment env) {
return env.osm.getNumKeys();
return env.osm != null ? env.osm.getNumKeys() : 0;
}

/**
Expand All @@ -1270,7 +1277,7 @@ public static int number_of_tags(Environment env) {
* @return the value of the setting (calculated when the style is loaded)
*/
public static Object setting(Environment env, String key) {
return env.source.settingValues.get(key);
return env.source != null ? env.source.settingValues.get(key) : null;
}

/**
Expand All @@ -1280,7 +1287,12 @@ public static Object setting(Environment env, String key) {
* @since 11247
*/
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;
}

/**
Expand Down Expand Up @@ -1315,7 +1327,11 @@ public static boolean outside(Environment env, String codes) {
* @since 12514
*/
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;
}

/**
Expand Down
57 changes: 57 additions & 0 deletions test/unit/org/openstreetmap/josm/gui/mappaint/mapcss/FunctionsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,28 @@
package org.openstreetmap.josm.gui.mappaint.mapcss;

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;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
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;
import org.openstreetmap.josm.data.osm.IPrimitive;
Expand All @@ -29,11 +37,13 @@
import org.openstreetmap.josm.data.osm.Way;
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;
import org.openstreetmap.josm.testutils.annotations.BasicPreferences;
import org.openstreetmap.josm.testutils.annotations.MapPaintStyles;
import org.openstreetmap.josm.testutils.annotations.Projection;
import org.openstreetmap.josm.testutils.annotations.Territories;

/**
* Unit tests of {@link Functions}.
Expand Down Expand Up @@ -63,6 +73,22 @@ Environment build() {
}
}

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;
}

/**
* Unit test of {@link Functions#title}.
*/
Expand Down Expand Up @@ -230,4 +256,35 @@ void testParentOsmPrimitives() {

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));
}
}
}

0 comments on commit 8203bfd

Please sign in to comment.