Skip to content

Commit 920c417

Browse files
committed
Switch to iterative version of WKT format parser
Signed-off-by: Heemin Kim <[email protected]>
1 parent 10c0b77 commit 920c417

File tree

3 files changed

+99
-6
lines changed

3 files changed

+99
-6
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
1717
### Removed
1818

1919
### Fixed
20+
- Switch to iterative version of WKT format parser ([#14086](https://github.com/opensearch-project/OpenSearch/pull/14086))
2021

2122
### Security
2223

libs/geo/src/main/java/org/opensearch/geometry/utils/WellKnownText.java

Lines changed: 68 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,10 @@
4949
import java.io.StreamTokenizer;
5050
import java.io.StringReader;
5151
import java.text.ParseException;
52+
import java.util.ArrayDeque;
5253
import java.util.ArrayList;
5354
import java.util.Collections;
55+
import java.util.Deque;
5456
import java.util.List;
5557
import java.util.Locale;
5658

@@ -67,6 +69,7 @@ public class WellKnownText {
6769
public static final String RPAREN = ")";
6870
public static final String COMMA = ",";
6971
public static final String NAN = "NaN";
72+
public static final int MAX_DEPTH_OF_GEO_COLLECTION = 1000;
7073

7174
private final String NUMBER = "<NUMBER>";
7275
private final String EOF = "END-OF-STREAM";
@@ -278,6 +281,16 @@ public Geometry fromWKT(String wkt) throws IOException, ParseException {
278281
*/
279282
private Geometry parseGeometry(StreamTokenizer stream) throws IOException, ParseException {
280283
final String type = nextWord(stream).toLowerCase(Locale.ROOT);
284+
switch (type) {
285+
case "geometrycollection":
286+
return parseGeometryCollection(stream);
287+
default:
288+
return parseSimpleGeometry(stream, type);
289+
}
290+
}
291+
292+
private Geometry parseSimpleGeometry(StreamTokenizer stream, String type) throws IOException, ParseException {
293+
assert "geometrycollection".equals(type) == false;
281294
switch (type) {
282295
case "point":
283296
return parsePoint(stream);
@@ -294,23 +307,72 @@ private Geometry parseGeometry(StreamTokenizer stream) throws IOException, Parse
294307
case "bbox":
295308
return parseBBox(stream);
296309
case "geometrycollection":
297-
return parseGeometryCollection(stream);
310+
throw new IllegalStateException("Unexpected type: geometrycollection");
298311
case "circle": // Not part of the standard, but we need it for internal serialization
299312
return parseCircle(stream);
300313
}
301314
throw new IllegalArgumentException("Unknown geometry type: " + type);
302315
}
303316

317+
/**
318+
* Parse geometry collection iteratively
319+
*
320+
* Parsing geometry collection recursively can lead to StackOverflowError when there is a deeply nested structure of GeometryCollection
321+
*/
304322
private GeometryCollection<Geometry> parseGeometryCollection(StreamTokenizer stream) throws IOException, ParseException {
305323
if (nextEmptyOrOpen(stream).equals(EMPTY)) {
306324
return GeometryCollection.EMPTY;
307325
}
308-
List<Geometry> shapes = new ArrayList<>();
309-
shapes.add(parseGeometry(stream));
310-
while (nextCloserOrComma(stream).equals(COMMA)) {
311-
shapes.add(parseGeometry(stream));
326+
327+
List<Geometry> topLevelShapes = new ArrayList<>();
328+
Deque<List<Geometry>> deque = new ArrayDeque<>();
329+
deque.push(topLevelShapes);
330+
boolean isFirstIteration = true;
331+
List<Geometry> currentLevelShapes = null;
332+
while (!deque.isEmpty()) {
333+
List<Geometry> previousShapes = deque.pop();
334+
if (currentLevelShapes != null) {
335+
previousShapes.add(new GeometryCollection<>(currentLevelShapes));
336+
}
337+
currentLevelShapes = previousShapes;
338+
339+
if (isFirstIteration == true) {
340+
isFirstIteration = false;
341+
} else {
342+
if (nextCloserOrComma(stream).equals(COMMA) == false) {
343+
// Done with current level, continue with parent level
344+
continue;
345+
}
346+
}
347+
while (true) {
348+
final String type = nextWord(stream).toLowerCase(Locale.ROOT);
349+
if (type.equals("geometrycollection")) {
350+
if (nextEmptyOrOpen(stream).equals(EMPTY) == false) {
351+
// GEOMETRYCOLLECTION() -> 1 depth, GEOMETRYCOLLECTION(GEOMETRYCOLLECTION()) -> 2 depth
352+
// When parsing the top level geometry collection, the queue size is zero.
353+
// When max depth is 1, we don't want to push any sub geometry collection in the queue.
354+
// Therefore, we subtract 2 from max depth.
355+
if (deque.size() >= MAX_DEPTH_OF_GEO_COLLECTION - 2) {
356+
throw new IllegalArgumentException(
357+
"a geometry collection with a depth greater than " + MAX_DEPTH_OF_GEO_COLLECTION + " is not supported"
358+
);
359+
}
360+
deque.push(currentLevelShapes);
361+
currentLevelShapes = new ArrayList<>();
362+
continue;
363+
}
364+
currentLevelShapes.add(GeometryCollection.EMPTY);
365+
} else {
366+
currentLevelShapes.add(parseSimpleGeometry(stream, type));
367+
}
368+
369+
if (nextCloserOrComma(stream).equals(COMMA) == false) {
370+
break;
371+
}
372+
}
312373
}
313-
return new GeometryCollection<>(shapes);
374+
375+
return new GeometryCollection<>(topLevelShapes);
314376
}
315377

316378
private Point parsePoint(StreamTokenizer stream) throws IOException, ParseException {

libs/geo/src/test/java/org/opensearch/geometry/GeometryCollectionTests.java

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,11 @@ public void testBasicSerialization() throws IOException, ParseException {
6262

6363
assertEquals("GEOMETRYCOLLECTION EMPTY", wkt.toWKT(GeometryCollection.EMPTY));
6464
assertEquals(GeometryCollection.EMPTY, wkt.fromWKT("GEOMETRYCOLLECTION EMPTY)"));
65+
66+
assertEquals(
67+
new GeometryCollection<Geometry>(Arrays.asList(GeometryCollection.EMPTY)),
68+
wkt.fromWKT("GEOMETRYCOLLECTION (GEOMETRYCOLLECTION EMPTY)")
69+
);
6570
}
6671

6772
@SuppressWarnings("ConstantConditions")
@@ -86,4 +91,29 @@ public void testInitValidation() {
8691

8792
new StandardValidator(true).validate(new GeometryCollection<Geometry>(Collections.singletonList(new Point(20, 10, 30))));
8893
}
94+
95+
public void testDeeplyNestedGeometryCollection() throws IOException, ParseException {
96+
WellKnownText wkt = new WellKnownText(true, new GeographyValidator(true));
97+
StringBuilder validGeometryCollectionHead = new StringBuilder("GEOMETRYCOLLECTION");
98+
StringBuilder validGeometryCollectionTail = new StringBuilder(" EMPTY");
99+
for (int i = 0; i < WellKnownText.MAX_DEPTH_OF_GEO_COLLECTION - 1; i++) {
100+
validGeometryCollectionHead.append(" (GEOMETRYCOLLECTION");
101+
validGeometryCollectionTail.append(")");
102+
}
103+
// Expect no exception
104+
wkt.fromWKT(validGeometryCollectionHead.append(validGeometryCollectionTail).toString());
105+
106+
StringBuilder invalidGeometryCollectionHead = new StringBuilder("GEOMETRYCOLLECTION");
107+
StringBuilder invalidGeometryCollectionTail = new StringBuilder(" EMPTY");
108+
for (int i = 0; i < WellKnownText.MAX_DEPTH_OF_GEO_COLLECTION; i++) {
109+
invalidGeometryCollectionHead.append(" (GEOMETRYCOLLECTION");
110+
invalidGeometryCollectionTail.append(")");
111+
}
112+
113+
IllegalArgumentException ex = expectThrows(
114+
IllegalArgumentException.class,
115+
() -> wkt.fromWKT(invalidGeometryCollectionHead.append(invalidGeometryCollectionTail).toString())
116+
);
117+
assertEquals("a geometry collection with a depth greater than 1000 is not supported", ex.getMessage());
118+
}
89119
}

0 commit comments

Comments
 (0)