Changeset 18619 in josm

Timestamp:

2022-12-27T16:51:43+01:00 (2 years ago)

Author:

taylor.smock

Message:

Fix #22600: NPE: cannot invoke "EastNorth.isValid" because "common" is null

Location:

trunk

Files:

• src/org/openstreetmap/josm/data/validation/tests/SharpAngles.java (modified) (1 diff)
• test/unit/org/openstreetmap/josm/data/validation/tests/SharpAnglesTest.java (modified) (7 diffs)

trunk/src/org/openstreetmap/josm/data/validation/tests/SharpAngles.java

@@ -92 +92 @@
 
     private void checkAngle(Node node1, Node node2, Node node3, int i, Way way, boolean last) {
-        if (node1 == null || node2 == null || node3 == null) return;
+        if (node1 == null || !node1.isLatLonKnown()
+            || node2 == null || !node2.isLatLonKnown()
+            || node3 == null || !node3.isLatLonKnown()) {
+            return;
+        }
         EastNorth n1 = node1.getEastNorth();
         EastNorth n2 = node2.getEastNorth();

trunk/test/unit/org/openstreetmap/josm/data/validation/tests/SharpAnglesTest.java

@@ -2 +2 @@
 package org.openstreetmap.josm.data.validation.tests;
 
-import org.junit.Assert;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.util.Arrays;
+import java.util.stream.Stream;
+
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 import org.openstreetmap.josm.JOSMFixture;
 import org.openstreetmap.josm.TestUtils;
+import org.openstreetmap.josm.data.coor.EastNorth;
 import org.openstreetmap.josm.data.coor.LatLon;
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.Way;
+import org.openstreetmap.josm.testutils.annotations.BasicPreferences;
 
 /**
  * JUnit Test of the Sharp Angles validation test.
  */
+@BasicPreferences
 class SharpAnglesTest {
     private SharpAngles angles;
@@ -38 +49 @@
         way.addNode(way.firstNode());
         angles.visit(way);
-        Assert.assertEquals(0, angles.getErrors().size());
+        assertEquals(0, angles.getErrors().size());
     }
 
@@ -52 +63 @@
         angles.setMaxLength(Double.MAX_VALUE);
         angles.visit(way);
-        Assert.assertEquals(1, angles.getErrors().size());
+        assertEquals(1, angles.getErrors().size());
     }
 
@@ -67 +78 @@
         angles.setMaxLength(Double.MAX_VALUE);
         angles.visit(way);
-        Assert.assertEquals(2, angles.getErrors().size());
+        assertEquals(2, angles.getErrors().size());
     }
 
@@ -79 +90 @@
                 new Node(new LatLon(0.2, 0.3)), new Node(new LatLon(0.3, 0.1)));
         angles.visit(way);
-        Assert.assertEquals(0, angles.getErrors().size());
+        assertEquals(0, angles.getErrors().size());
     }
 
@@ -98 +109 @@
         way.addNode(way.firstNode());
         angles.visit(way);
-        Assert.assertEquals(0, angles.getErrors().size());
+        assertEquals(0, angles.getErrors().size());
     }
 
@@ -111 +122 @@
         angles.setMaxLength(Double.MAX_VALUE);
         angles.visit(way);
-        Assert.assertEquals(1, angles.getErrors().size());
+        assertEquals(1, angles.getErrors().size());
         angles.getErrors().clear();
 
         way.put("highway", "rest_area");
         angles.visit(way);
-        Assert.assertEquals(0, angles.getErrors().size());
+        assertEquals(0, angles.getErrors().size());
 
         way.put("highway", "residential");
         angles.visit(way);
-        Assert.assertEquals(1, angles.getErrors().size());
+        assertEquals(1, angles.getErrors().size());
         angles.getErrors().clear();
         way.put("area", "yes");
         angles.visit(way);
-        Assert.assertEquals(0, angles.getErrors().size());
+        assertEquals(0, angles.getErrors().size());
         way.put("area", "no");
         angles.visit(way);
-        Assert.assertEquals(1, angles.getErrors().size());
+        assertEquals(1, angles.getErrors().size());
+    }
+
+    static Stream<Arguments> testNonRegression22600() {
+        return Stream.of(
+                // All invalid nodes
+                Arguments.of(new Node(), new Node(), new Node()),
+                // Single valid nodes
+                Arguments.of(new Node(LatLon.SOUTH_POLE), new Node(), new Node()),
+                Arguments.of(new Node(), new Node(LatLon.ZERO), new Node()),
+                Arguments.of(new Node(), new Node(), new Node(LatLon.NORTH_POLE)),
+                // Two valid nodes
+                Arguments.of(new Node(), new Node(LatLon.ZERO), new Node(LatLon.NORTH_POLE)),
+                Arguments.of(new Node(LatLon.SOUTH_POLE), new Node(LatLon.ZERO), new Node()),
+                Arguments.of(new Node(LatLon.SOUTH_POLE), new Node(), new Node(LatLon.NORTH_POLE)),
+                // All valid nodes
+                Arguments.of(new Node(LatLon.SOUTH_POLE), new Node(LatLon.ZERO), new Node(LatLon.NORTH_POLE))
+        );
+    }
+
+    /**
+     * Non-regression test for #22600: NPE: Cannot invoke {@link EastNorth#isValid()} because "common" is null
+     */
+    @ParameterizedTest
+    @MethodSource
+    void testNonRegression22600(Node node1, Node node2, Node node3) {
+        Way invalidWay = new Way();
+        invalidWay.put("highway", "trunk");
+        invalidWay.setNodes(Arrays.asList(node1, node2, node3));
+        assertDoesNotThrow(() -> this.angles.visit(invalidWay));
+        for (Node node : invalidWay.getNodes()) {
+            node.setCoor(LatLon.ZERO);
+            assertDoesNotThrow(() -> this.angles.visit(invalidWay));
+        }
     }
 }