Skip to content

Commit 012834f

Browse files
lvcalvca
committed
fix: point() over a latitude property surfaced as string may trigger internal ClassCastException
Fixed issue #4305
1 parent 2550f6a commit 012834f

2 files changed

Lines changed: 214 additions & 12 deletions

File tree

‎engine/src/main/java/com/arcadedb/function/geo/CypherPointFunction.java‎

Lines changed: 48 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,11 @@
3737
* </ul>
3838
* <p>The returned map contains the coordinate keys and a {@code crs} field indicating
3939
* the coordinate reference system.</p>
40+
* <p>Numeric coordinate keys that resolve to a {@link String} are coerced to {@link Number}
41+
* when the string parses cleanly as a decimal (e.g. a node whose {@code lat} property was
42+
* declared as {@link com.arcadedb.schema.Type#STRING}). When coercion is impossible the
43+
* function raises a {@link CommandExecutionException} naming the offending key and value
44+
* rather than leaking a raw {@link ClassCastException} (issue #4305).</p>
4045
*/
4146
public class CypherPointFunction implements StatelessFunction {
4247
@Override
@@ -53,10 +58,8 @@ public Object execute(final Object[] args, final CommandContext context) {
5358
if (args.length == 2) {
5459
if (args[0] == null || args[1] == null)
5560
return null;
56-
if (!(args[0] instanceof Number) || !(args[1] instanceof Number))
57-
throw new CommandExecutionException("point() with two arguments requires numeric latitude and longitude");
58-
final double lat = ((Number) args[0]).doubleValue();
59-
final double lon = ((Number) args[1]).doubleValue();
61+
final double lat = coerceCoordinate("latitude", args[0]);
62+
final double lon = coerceCoordinate("longitude", args[1]);
6063
final Map<String, Object> result = new LinkedHashMap<>();
6164
result.put("latitude", lat);
6265
result.put("longitude", lon);
@@ -81,8 +84,8 @@ public Object execute(final Object[] args, final CommandContext context) {
8184
final Object lat = map.get("latitude");
8285
if (lon == null || lat == null)
8386
return null;
84-
final double x = ((Number) lon).doubleValue();
85-
final double y = ((Number) lat).doubleValue();
87+
final double x = coerceCoordinate("longitude", lon);
88+
final double y = coerceCoordinate("latitude", lat);
8689
result.put("longitude", x);
8790
result.put("latitude", y);
8891
result.put("x", x);
@@ -98,8 +101,8 @@ public Object execute(final Object[] args, final CommandContext context) {
98101
final Object yv = map.get("y");
99102
if (xv == null || yv == null)
100103
return null;
101-
final double x = ((Number) xv).doubleValue();
102-
final double y = ((Number) yv).doubleValue();
104+
final double x = coerceCoordinate("x", xv);
105+
final double y = coerceCoordinate("y", yv);
103106
result.put("x", x);
104107
result.put("y", y);
105108
addOptionalZ(result, map);
@@ -108,8 +111,13 @@ public Object execute(final Object[] args, final CommandContext context) {
108111
result.put("crs", crsObj.toString());
109112
else
110113
result.put("crs", result.containsKey("z") ? "cartesian-3D" : "cartesian");
111-
if (map.containsKey("srid"))
112-
result.put("srid", ((Number) map.get("srid")).intValue());
114+
if (map.containsKey("srid")) {
115+
final Object sridObj = map.get("srid");
116+
if (!(sridObj instanceof Number))
117+
throw new CommandExecutionException(
118+
"point() 'srid' must be numeric, found " + describe(sridObj));
119+
result.put("srid", ((Number) sridObj).intValue());
120+
}
113121
} else {
114122
throw new CommandExecutionException("point() map must contain x/y or longitude/latitude properties");
115123
}
@@ -121,11 +129,39 @@ private void addOptionalZ(final Map<String, Object> result, final Map<?, ?> map)
121129
if (map.containsKey("z")) {
122130
final Object zv = map.get("z");
123131
if (zv != null)
124-
result.put("z", ((Number) zv).doubleValue());
132+
result.put("z", coerceCoordinate("z", zv));
125133
} else if (map.containsKey("height")) {
126134
final Object hv = map.get("height");
127135
if (hv != null)
128-
result.put("z", ((Number) hv).doubleValue());
136+
result.put("z", coerceCoordinate("height", hv));
129137
}
130138
}
139+
140+
/**
141+
* Returns the numeric value of {@code value}, coercing a numeric {@link String} when the
142+
* underlying property is typed as STRING but the contents are a clean decimal literal.
143+
* Throws a {@link CommandExecutionException} that names {@code key} and {@code value}
144+
* when coercion is impossible, so the user sees a clear error instead of a raw
145+
* {@link ClassCastException} (issue #4305).
146+
*/
147+
private static double coerceCoordinate(final String key, final Object value) {
148+
if (value instanceof Number n)
149+
return n.doubleValue();
150+
if (value instanceof String s) {
151+
try {
152+
return Double.parseDouble(s.trim());
153+
} catch (final NumberFormatException ignored) {
154+
throw new CommandExecutionException(
155+
"point() '" + key + "' must be numeric, found String value '" + s + "'");
156+
}
157+
}
158+
throw new CommandExecutionException(
159+
"point() '" + key + "' must be numeric, found " + describe(value));
160+
}
161+
162+
private static String describe(final Object value) {
163+
if (value == null)
164+
return "null";
165+
return value.getClass().getSimpleName() + " value '" + value + "'";
166+
}
131167
}

‎engine/src/test/java/com/arcadedb/query/opencypher/Issue4305Test.java‎

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
/*
2+
* Copyright 2021-present Arcade Data Ltd (info@arcadedata.com)
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*
16+
* SPDX-FileCopyrightText: 2021-present Arcade Data Ltd (info@arcadedata.com)
17+
* SPDX-License-Identifier: Apache-2.0
18+
*/
19+
package com.arcadedb.query.opencypher;
20+
21+
import com.arcadedb.database.Database;
22+
import com.arcadedb.database.DatabaseFactory;
23+
import com.arcadedb.exception.CommandExecutionException;
24+
import com.arcadedb.query.sql.executor.Result;
25+
import com.arcadedb.query.sql.executor.ResultSet;
26+
import com.arcadedb.schema.Type;
27+
import com.arcadedb.schema.VertexType;
28+
import org.junit.jupiter.api.AfterEach;
29+
import org.junit.jupiter.api.BeforeEach;
30+
import org.junit.jupiter.api.Test;
31+
32+
import java.util.Map;
33+
34+
import static org.assertj.core.api.Assertions.assertThat;
35+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
36+
37+
/**
38+
* Reproducer for <a href="https://github.com/ArcadeData/arcadedb/issues/4305">issue #4305</a>:
39+
* point() over a latitude property surfaced as String triggered an internal ClassCastException.
40+
* <p>
41+
* The user's scenario is a database where the {@code City.lat} property is declared as
42+
* {@link Type#STRING} (or otherwise gets stored as a String). When they call
43+
* {@code point({latitude: c.lat, longitude: c.lon})}, the function fails with a raw
44+
* {@link ClassCastException} from {@code java.lang.String cannot be cast to java.lang.Number}.
45+
* The fix coerces numeric strings to {@link Number} and otherwise raises a clean
46+
* {@link CommandExecutionException} naming the offending key.
47+
*
48+
* @author Luca Garulli (l.garulli@arcadedata.com)
49+
*/
50+
class Issue4305Test {
51+
private Database database;
52+
53+
@BeforeEach
54+
void setUp() {
55+
final DatabaseFactory factory = new DatabaseFactory("./databases/issue4305");
56+
if (factory.exists())
57+
factory.open().drop();
58+
database = factory.create();
59+
}
60+
61+
@AfterEach
62+
void tearDown() {
63+
if (database != null && database.isOpen())
64+
database.drop();
65+
}
66+
67+
@Test
68+
void propertyNamedLatStaysNumeric() {
69+
database.command("opencypher", "CREATE (:City {lat: 1.0, lon: 2.0})");
70+
71+
try (final ResultSet rs = database.query("opencypher",
72+
"MATCH (c:City) RETURN c.lat AS lat, c.lon AS lon")) {
73+
assertThat(rs.hasNext()).isTrue();
74+
final Result row = rs.next();
75+
assertThat(row.<Object>getProperty("lat")).isInstanceOf(Number.class);
76+
assertThat(row.<Object>getProperty("lon")).isInstanceOf(Number.class);
77+
}
78+
}
79+
80+
@Test
81+
void pointFromLatLonProperties() {
82+
database.command("opencypher", "CREATE (:City {lat: 1.0, lon: 2.0})");
83+
84+
try (final ResultSet rs = database.query("opencypher",
85+
"MATCH (c:City) RETURN point({latitude: c.lat, longitude: c.lon}) AS p")) {
86+
assertThat(rs.hasNext()).isTrue();
87+
final Object point = rs.next().getProperty("p");
88+
assertThat(point).isInstanceOf(Map.class);
89+
final Map<?, ?> p = (Map<?, ?>) point;
90+
assertThat(((Number) p.get("latitude")).doubleValue()).isEqualTo(1.0);
91+
assertThat(((Number) p.get("longitude")).doubleValue()).isEqualTo(2.0);
92+
}
93+
}
94+
95+
/**
96+
* The actual reproducer for the reported ClassCastException: declare the schema with
97+
* {@code lat} as a STRING so writes are coerced to {@link String}. point() must coerce
98+
* a numeric string back to a number rather than crashing.
99+
*/
100+
@Test
101+
void pointCoercesNumericStringFromSchemaTypedStringProperty() {
102+
final VertexType city = database.getSchema().createVertexType("City");
103+
city.createProperty("lat", Type.STRING);
104+
city.createProperty("lon", Type.DOUBLE);
105+
database.command("opencypher", "CREATE (:City {lat: 1.0, lon: 2.0})");
106+
107+
try (final ResultSet check = database.query("opencypher",
108+
"MATCH (c:City) RETURN c.lat AS lat, c.lon AS lon")) {
109+
assertThat(check.hasNext()).isTrue();
110+
final Result row = check.next();
111+
// lat is stored as String because the schema declares it so - reproduce the user's state
112+
assertThat(row.<Object>getProperty("lat")).isInstanceOf(String.class);
113+
assertThat(row.<Object>getProperty("lon")).isInstanceOf(Number.class);
114+
}
115+
116+
try (final ResultSet rs = database.query("opencypher",
117+
"MATCH (c:City) RETURN point({latitude: c.lat, longitude: c.lon}) AS p")) {
118+
assertThat(rs.hasNext()).isTrue();
119+
final Object point = rs.next().getProperty("p");
120+
assertThat(point).as("point() must not crash on a numeric string and must return a usable point")
121+
.isInstanceOf(Map.class);
122+
final Map<?, ?> p = (Map<?, ?>) point;
123+
assertThat(((Number) p.get("latitude")).doubleValue()).isEqualTo(1.0);
124+
assertThat(((Number) p.get("longitude")).doubleValue()).isEqualTo(2.0);
125+
}
126+
}
127+
128+
/**
129+
* When the value cannot be coerced (non-numeric string), point() must raise a clean,
130+
* user-facing error naming the bad key - never a raw ClassCastException.
131+
*/
132+
@Test
133+
void pointRaisesCleanErrorOnNonNumericString() {
134+
final VertexType city = database.getSchema().createVertexType("Place");
135+
city.createProperty("lat", Type.STRING);
136+
city.createProperty("lon", Type.STRING);
137+
database.command("opencypher", "CREATE (:Place {lat: 'abc', lon: 'def'})");
138+
139+
assertThatThrownBy(() -> {
140+
try (final ResultSet rs = database.query("opencypher",
141+
"MATCH (p:Place) RETURN point({latitude: p.lat, longitude: p.lon}) AS pt")) {
142+
rs.stream().toList();
143+
}
144+
})
145+
.isInstanceOf(CommandExecutionException.class)
146+
.hasMessageContaining("point()")
147+
.hasMessageMatching("(?s).*(latitude|longitude).*")
148+
.hasMessageMatching("(?s).*(abc|def).*");
149+
}
150+
151+
@Test
152+
void pointFromXvYvPropertiesControl() {
153+
// From the issue: using property names other than lat/lon works fine.
154+
database.command("opencypher", "CREATE (:Town {xv: 1.0, yv: 2.0})");
155+
156+
try (final ResultSet rs = database.query("opencypher",
157+
"MATCH (c:Town) RETURN point({latitude: c.xv, longitude: c.yv}) AS p")) {
158+
assertThat(rs.hasNext()).isTrue();
159+
final Object point = rs.next().getProperty("p");
160+
assertThat(point).isInstanceOf(Map.class);
161+
final Map<?, ?> p = (Map<?, ?>) point;
162+
assertThat(((Number) p.get("latitude")).doubleValue()).isEqualTo(1.0);
163+
assertThat(((Number) p.get("longitude")).doubleValue()).isEqualTo(2.0);
164+
}
165+
}
166+
}

0 commit comments

Comments
 (0)