From 3388d616c3b3ba28e03b7c0463b9d293ebe0a638 Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Wed, 3 Jun 2026 13:11:35 +1000 Subject: [PATCH 1/2] MDEV-39813 ST_GeomFromGeoJSON does not control recursion depth Geometry::create_from_json used strings as things that reinitialize the je (json_engine). By reinitializing with json_start the killed pointer gets reset, and also the concept of depth is reset. There are two states in create_from_json: 1. The search if for the type, once found ci is set 2. Search for the argument, arg becomes != T_NONE If the type is found first, ci become the class, and once the arg is found, continued execution on the same je occurs. If the argument comes before the "type", we've save a copy of the json_engine_t to process this argument. In both cases je after the execution is the most progressed pointer. Debug assertions on je state aren't enforced by code, if some invalid GeoJSON is passed, these are actual errors. Because Geometry::create_from_json is recursive, from geometrymcollections, a json_read_value is pushed from inside this function up to Item_func_geometry_from_json::val_str. Other Gis*::init_from_json start from their actual object and don't need a json_read_value to start. Test case an bug report thanks to byteoverride. --- mysql-test/main/gis-json.result | 80 +++++++++++- mysql-test/main/gis-json.test | 77 ++++++++++++ sql/item_geofunc.cc | 3 +- sql/spatial.cc | 215 ++++++++++++++++++++------------ 4 files changed, 291 insertions(+), 84 deletions(-) diff --git a/mysql-test/main/gis-json.result b/mysql-test/main/gis-json.result index 6e55b6d88e2eb..e636291b5db95 100644 --- a/mysql-test/main/gis-json.result +++ b/mysql-test/main/gis-json.result @@ -60,7 +60,9 @@ exp POINT(102 0.5) SELECT st_astext(st_geomfromgeojson('{ "geometry": { "type": "Point", "coordinates": [102.0, 0.5] }, "type": "Feature" }')) as exp; exp -POINT(102 0.5) +NULL +Warnings: +Warning 4048 Incorrect GeoJSON format specified for st_geomfromgeojson function. SELECT st_astext(st_geomfromgeojson('{ "type": "FeatureCollection", "features": [{ "type": "Feature", "geometry": { "type": "Point", "coordinates": [102.0, 0.5] }, "properties": { "prop0": "value0" } }]}')) as exp; exp GEOMETRYCOLLECTION(POINT(102 0.5)) @@ -110,6 +112,8 @@ Warning 4076 Incorrect GeoJSON format - empty 'coordinates' array. SELECT ST_GEOMFROMGEOJSON("{ \"type\": \"Feature\", \"geometry\": [10, 20] }") as exp; exp NULL +Warnings: +Warning 4048 Incorrect GeoJSON format specified for st_geomfromgeojson function. SELECT ST_ASTEXT (ST_GEOMFROMGEOJSON ('{ "type": "GEOMETRYCOLLECTION", "coordinates": [102.0, 0.0]}')) as exp; exp NULL @@ -135,5 +139,77 @@ repeat(']}', 2000) exp NULL Warnings: -Warning 4040 Limit of 32 on JSON nested structures depth is reached in argument 1 to function 'st_geomfromgeojson' at position 473 +Warning 4040 Limit of 32 on JSON nested structures depth is reached in argument 1 to function 'st_geomfromgeojson' at position 688 +# Boundary condition: foreign-member nesting at depth 32 +SET @geom := '{"type":"Point","coordinates":[1,2]}'; +SET @doc := CONCAT( +REPEAT('{"x":', 29), +@geom, +REPEAT('}', 29) +); +SELECT ST_ASTEXT( ST_GeomFromGeoJSON(@doc) ) AS expr; +expr +NULL +Warnings: +Warning 4048 Incorrect GeoJSON format specified for st_geomfromgeojson function. +# Foreign-member nesting exceeds depth 32 +SET @geom := '{"type":"Point","coordinates":[1,2]}'; +SET @doc := CONCAT( +REPEAT('{"x":', 30), +@geom, +REPEAT('}', 30) +); +SELECT ST_GeomFromGeoJSON(@doc) AS expr; +expr +NULL +Warnings: +Warning 4040 Limit of 32 on JSON nested structures depth is reached in argument 1 to function 'st_geomfromgeojson' at position 181 +# Deep foreign member attached to otherwise valid GeoJSON +SET @foreign := CONCAT( +REPEAT('{"x":', 28), +'{"leaf":1}', +REPEAT('}', 28) +); +SET @doc := CONCAT( +'{"type":"Point","coordinates":[10,20],"foreign":', +@foreign, +'}' +); +SELECT ST_ASTEXT(ST_GeomFromGeoJSON(@doc)) AS expr; +expr +POINT(10 20) +# Foreign member exceeds depth limit +SET @foreign := CONCAT( +REPEAT('{"x":', 29), +'{"leaf":1}', +REPEAT('}', 29) +); +SET @doc := CONCAT( +'{"type":"Point","coordinates":[10,20],"foreign":', +@foreign, +'}' +); +SELECT ST_ASTEXT(ST_GeomFromGeoJSON(@doc)) AS expr; +expr +POINT(10 20) +# GeometryCollection containing 33 valid GeoJSON Point objects +SET @point := '{"type":"Point","coordinates":[1,2]}'; +SET @geoms := CONCAT( +@point, ',', @point, ',', @point, ',', @point, ',', @point, ',', +@point, ',', @point, ',', @point, ',', @point, ',', @point, ',', +@point, ',', @point, ',', @point, ',', @point, ',', @point, ',', +@point, ',', @point, ',', @point, ',', @point, ',', @point, ',', +@point, ',', @point, ',', @point, ',', @point, ',', @point, ',', +@point, ',', @point, ',', @point, ',', @point, ',', @point, ',', +@point, ',', @point, ',', @point, ',', @point, ',', @point, ',', +@point, ',', @point, ',', @point +); +SET @doc := CONCAT( +'{"type":"GeometryCollection","geometries":[', +@geoms, +']}' +); +SELECT ST_AsText(ST_GeomFromGeoJSON(@doc)) AS expr; +expr +GEOMETRYCOLLECTION(POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2)) # End of 10.6 tests diff --git a/mysql-test/main/gis-json.test b/mysql-test/main/gis-json.test index f635e2c6ec7d0..33f00ee414459 100644 --- a/mysql-test/main/gis-json.test +++ b/mysql-test/main/gis-json.test @@ -70,4 +70,81 @@ SELECT ST_GeomFromGeoJSON( repeat(']}', 2000) )) as exp; +--echo # Boundary condition: foreign-member nesting at depth 32 +SET @geom := '{"type":"Point","coordinates":[1,2]}'; + +SET @doc := CONCAT( + REPEAT('{"x":', 29), + @geom, + REPEAT('}', 29) +); + +SELECT ST_ASTEXT( ST_GeomFromGeoJSON(@doc) ) AS expr; + + +--echo # Foreign-member nesting exceeds depth 32 +SET @geom := '{"type":"Point","coordinates":[1,2]}'; + +SET @doc := CONCAT( + REPEAT('{"x":', 30), + @geom, + REPEAT('}', 30) +); + +SELECT ST_GeomFromGeoJSON(@doc) AS expr; + + +--echo # Deep foreign member attached to otherwise valid GeoJSON +SET @foreign := CONCAT( + REPEAT('{"x":', 28), + '{"leaf":1}', + REPEAT('}', 28) +); + +SET @doc := CONCAT( + '{"type":"Point","coordinates":[10,20],"foreign":', + @foreign, + '}' +); + +SELECT ST_ASTEXT(ST_GeomFromGeoJSON(@doc)) AS expr; + + +--echo # Foreign member exceeds depth limit +SET @foreign := CONCAT( + REPEAT('{"x":', 29), + '{"leaf":1}', + REPEAT('}', 29) +); + +SET @doc := CONCAT( + '{"type":"Point","coordinates":[10,20],"foreign":', + @foreign, + '}' +); + +SELECT ST_ASTEXT(ST_GeomFromGeoJSON(@doc)) AS expr; + +--echo # GeometryCollection containing 33 valid GeoJSON Point objects +SET @point := '{"type":"Point","coordinates":[1,2]}'; + +SET @geoms := CONCAT( + @point, ',', @point, ',', @point, ',', @point, ',', @point, ',', + @point, ',', @point, ',', @point, ',', @point, ',', @point, ',', + @point, ',', @point, ',', @point, ',', @point, ',', @point, ',', + @point, ',', @point, ',', @point, ',', @point, ',', @point, ',', + @point, ',', @point, ',', @point, ',', @point, ',', @point, ',', + @point, ',', @point, ',', @point, ',', @point, ',', @point, ',', + @point, ',', @point, ',', @point, ',', @point, ',', @point, ',', + @point, ',', @point, ',', @point +); + +SET @doc := CONCAT( + '{"type":"GeometryCollection","geometries":[', + @geoms, + ']}' +); + +SELECT ST_AsText(ST_GeomFromGeoJSON(@doc)) AS expr; + --echo # End of 10.6 tests diff --git a/sql/item_geofunc.cc b/sql/item_geofunc.cc index cc54d06ec7b3f..deb2b1018f91f 100644 --- a/sql/item_geofunc.cc +++ b/sql/item_geofunc.cc @@ -163,7 +163,8 @@ String *Item_func_geometry_from_json::val_str(String *str) (const uchar *) js->end()); je.killed_ptr= (uint32_t *) ¤t_thd->killed; - if ((null_value= !Geometry::create_from_json(&buffer, &je, options==1, str))) + if (!json_read_value(&je) && + (null_value= !Geometry::create_from_json(&buffer, &je, options==1, str))) { int code= 0; diff --git a/sql/spatial.cc b/sql/spatial.cc index 79e36bf6215dc..ee808a000428e 100644 --- a/sql/spatial.cc +++ b/sql/spatial.cc @@ -493,19 +493,14 @@ Geometry *Geometry::create_from_json(Geometry_buffer *buffer, json_engine_t *je, bool er_on_3D, String *res) { Class_info *ci= NULL; - const uchar *coord_start= NULL, *geom_start= NULL, - *features_start= NULL, *geometry_start= NULL; + enum t_enum { T_NONE, T_GEOMETRY, T_GEOMETRIES, T_COORD, T_FEATURE }; + enum t_enum arg= T_NONE; + json_engine_t argje, *je_arg; Geometry *result; uchar key_buf[max_keyname_len]; uint key_len; - int fcoll_type_found= 0, feature_type_found= 0; + bool feature_type_found= false; - const uint32_t *killed_ptr= (uint32_t *) je->killed_ptr; - int stack_p; - - if (json_read_value(je)) - goto err_return; - if (je->value_type != JSON_VALUE_OBJECT) { je->s.error= GEOJ_INCORRECT_GEOJSON; @@ -514,7 +509,8 @@ Geometry *Geometry::create_from_json(Geometry_buffer *buffer, while (json_scan_next(je) == 0 && je->state != JST_OBJ_END) { - DBUG_ASSERT(je->state == JST_KEY); + if (je->state != JST_KEY) + break; key_len=0; while (json_read_keyname_chr(je) == 0) @@ -543,31 +539,36 @@ Geometry *Geometry::create_from_json(Geometry_buffer *buffer, if (je->value_type == JSON_VALUE_STRING) { - if ((ci= find_class((const char *) je->value, je->value_len))) + if ((ci= find_class(reinterpret_cast(je->value), je->value_len))) { - if ((coord_start= - (ci == &geometrycollection_class) ? geom_start : coord_start)) + if ((ci == &geometrycollection_class && arg == T_GEOMETRIES) || arg == T_COORD) goto create_geom; + if (arg != T_NONE) + break; /* invalid arg present for current geometry */ } else if (je->value_len == feature_coll_type_len && my_charset_latin1.strnncoll(je->value, je->value_len, - feature_coll_type, feature_coll_type_len) == 0) + feature_coll_type, feature_coll_type_len) == 0) { /* - 'FeatureCollection' type found. Handle the 'Featurecollection'/'features' - GeoJSON construction. + 'FeatureCollection' type found. Handle the 'Featurecollection' + /'features' GeoJSON construction. */ - if (features_start) - goto handle_feature_collection; - fcoll_type_found= 1; + ci= &geometrycollection_class; + if (arg == T_FEATURE) + goto create_geom; + if (arg != T_NONE) + break; /* invalid arg present for current geometry */ } else if (je->value_len == feature_type_len && my_charset_latin1.strnncoll(je->value, je->value_len, - feature_type, feature_type_len) == 0) + feature_type, feature_type_len) == 0) { - if (geometry_start) + if (arg == T_GEOMETRY) goto handle_geometry_key; - feature_type_found= 1; + feature_type_found= true; + if (arg != T_NONE) + break; /* invalid arg present for current geometry */ } else /* can't understand the type. */ break; @@ -578,6 +579,8 @@ Geometry *Geometry::create_from_json(Geometry_buffer *buffer, else if (key_len == coord_keyname_len && memcmp(key_buf, coord_keyname, coord_keyname_len) == 0) { + if (arg != T_NONE) + break; /* previous arg unprocessed */ /* Found the "coordinates" key. Let's check it's an array and remember where it starts. @@ -587,16 +590,25 @@ Geometry *Geometry::create_from_json(Geometry_buffer *buffer, if (je->value_type == JSON_VALUE_ARRAY) { - coord_start= je->value_begin; + arg= T_COORD; if (ci && ci != &geometrycollection_class) - goto create_geom; + { + je_arg= je; + goto create_geom; + } + argje= *je; + je_arg= &argje; if (json_skip_level(je)) goto err_return; } + else + break; /* coordinates needs to be an array */ } else if (key_len == geometries_keyname_len && memcmp(key_buf, geometries_keyname, geometries_keyname_len) == 0) { + if (arg != T_NONE) + break; /* previous arg unprocessed */ /* Found the "geometries" key. Let's check it's an array and remember where it starts. @@ -606,17 +618,25 @@ Geometry *Geometry::create_from_json(Geometry_buffer *buffer, if (je->value_type == JSON_VALUE_ARRAY) { - geom_start= je->value_begin; if (ci == &geometrycollection_class) { - coord_start= geom_start; + je_arg= je; goto create_geom; } + if (ci != nullptr) + break; /* geometries only valid inside geometrycollation */ + arg= T_GEOMETRIES; + argje= *je; + je_arg= &argje; } + else + break; /* geometries needs to be an array */ } else if (key_len == features_keyname_len && memcmp(key_buf, features_keyname, features_keyname_len) == 0) { + if (arg != T_NONE) + break; /* previous arg unprocessed */ /* 'features' key found. Handle the 'Featurecollection'/'features' GeoJSON construction. @@ -625,10 +645,20 @@ Geometry *Geometry::create_from_json(Geometry_buffer *buffer, goto err_return; if (je->value_type == JSON_VALUE_ARRAY) { - features_start= je->value_begin; - if (fcoll_type_found) - goto handle_feature_collection; + if (ci == &geometrycollection_class) + { + je_arg= je; + goto create_geom; + } + if (ci != nullptr) + break; /* features only valid inside featurecollation */ + + arg= T_FEATURE; + argje= *je; + je_arg= &argje; } + else + break; /* feature collections needs to be an array */ } else if (key_len == geometry_keyname_len && memcmp(key_buf, geometry_keyname, geometry_keyname_len) == 0) @@ -637,12 +667,19 @@ Geometry *Geometry::create_from_json(Geometry_buffer *buffer, goto err_return; if (je->value_type == JSON_VALUE_OBJECT) { - geometry_start= je->value_begin; if (feature_type_found) + { + je_arg= je; goto handle_geometry_key; + } + if (ci != nullptr) + break; /* geometry only valid inside feature */ + arg= T_GEOMETRY; + argje= *je; + je_arg= &argje; } else - goto err_return; + break; /* geometry needs to be an object */ } else { @@ -661,35 +698,35 @@ Geometry *Geometry::create_from_json(Geometry_buffer *buffer, } goto err_return; -handle_feature_collection: - ci= &geometrycollection_class; - coord_start= features_start; - create_geom: - stack_p= je->stack_p; - json_scan_start(je, je->s.cs, coord_start, je->s.str_end); - je->killed_ptr= killed_ptr; - je->stack_p= stack_p; - if (res->reserve(1 + 4, 512)) goto err_return; result= (*ci->m_create_func)(buffer->data); res->q_append((char) wkb_ndr); res->q_append((uint32) result->get_class_info()->m_type_id); - if (result->init_from_json(je, er_on_3D, res)) + if (result->init_from_json(je_arg, er_on_3D, res)) + { + if (je_arg != je) /* copy error out of copied engine */ + *je= *je_arg; + goto err_return; + } + /* finish of the object scan for validation/geomcollection */ + if (json_skip_level(je)) goto err_return; return result; -handle_geometry_key: - stack_p= je->stack_p; - json_scan_start(je, je->s.cs, geometry_start, je->s.str_end); - je->killed_ptr= killed_ptr; - je->stack_p= stack_p; - return create_from_json(buffer, je, er_on_3D, res); +handle_geometry_key: /* feature */ + result= create_from_json(buffer, je_arg, er_on_3D, res); + + /* skip rest of feature - can be arbitrary fields */ + if (json_skip_level(je)) + goto err_return; + + return result; err_return: return NULL; } @@ -961,7 +998,6 @@ static int read_point_from_json(json_engine_t *je, bool er_on_3D, while (json_scan_next(je) == 0 && je->state != JST_ARRAY_END) { - DBUG_ASSERT(je->state == JST_VALUE); if (json_read_value(je)) return 1; @@ -975,6 +1011,8 @@ static int read_point_from_json(json_engine_t *je, bool er_on_3D, n_coord++; } + if (je->s.error != 0) + return 1; if (n_coord <= 2 || !er_on_3D) return 0; je->s.error= Geometry::GEOJ_DIMENSION_NOT_SUPPORTED; @@ -988,8 +1026,6 @@ static int read_point_from_json(json_engine_t *je, bool er_on_3D, bool Gis_point::init_from_json(json_engine_t *je, bool er_on_3D, String *wkb) { double x, y; - if (json_read_value(je)) - return TRUE; if (je->value_type != JSON_VALUE_ARRAY) { @@ -1003,6 +1039,7 @@ bool Gis_point::init_from_json(json_engine_t *je, bool er_on_3D, String *wkb) wkb->q_append(x); wkb->q_append(y); + /* Note GEOJSON RFC7946 3.3.1 - 3D possible, but not WKB? */ return FALSE; } @@ -1279,11 +1316,9 @@ bool Gis_line_string::init_from_json(json_engine_t *je, bool er_on_3D, uint32 np_pos= wkb->length(); Gis_point p; - if (json_read_value(je)) - return TRUE; - if (je->value_type != JSON_VALUE_ARRAY) { +err_geoj_incorrect: je->s.error= GEOJ_INCORRECT_GEOJSON; return TRUE; } @@ -1294,7 +1329,13 @@ bool Gis_line_string::init_from_json(json_engine_t *je, bool er_on_3D, while (json_scan_next(je) == 0 && je->state != JST_ARRAY_END) { - DBUG_ASSERT(je->state == JST_VALUE); + if (je->state != JST_VALUE) + goto err_geoj_incorrect; + + if (json_read_value(je)) + return TRUE; + if (je->value_type != JSON_VALUE_ARRAY) + goto err_geoj_incorrect; if (p.init_from_json(je, er_on_3D, wkb)) return TRUE; @@ -1673,11 +1714,9 @@ bool Gis_polygon::init_from_json(json_engine_t *je, bool er_on_3D, String *wkb) uint32 lr_pos= wkb->length(); int closed; - if (json_read_value(je)) - return TRUE; - if (je->value_type != JSON_VALUE_ARRAY) { +err_geoj_incorrect: je->s.error= GEOJ_INCORRECT_GEOJSON; return TRUE; } @@ -1689,11 +1728,18 @@ bool Gis_polygon::init_from_json(json_engine_t *je, bool er_on_3D, String *wkb) while (json_scan_next(je) == 0 && je->state != JST_ARRAY_END) { Gis_line_string ls; - DBUG_ASSERT(je->state == JST_VALUE); + if (je->state != JST_VALUE) + goto err_geoj_incorrect; + + if (json_read_value(je)) + return TRUE; + if (je->value_type != JSON_VALUE_ARRAY) + goto err_geoj_incorrect; uint32 ls_pos=wkb->length(); if (ls.init_from_json(je, er_on_3D, wkb)) return TRUE; + ls.set_data_ptr(wkb->ptr() + ls_pos, wkb->length() - ls_pos); if (ls.is_closed(&closed) || !closed) { @@ -2189,11 +2235,9 @@ bool Gis_multi_point::init_from_json(json_engine_t *je, bool er_on_3D, uint32 np_pos= wkb->length(); Gis_point p; - if (json_read_value(je)) - return TRUE; - if (je->value_type != JSON_VALUE_ARRAY) { +err_geoj_incorrect: je->s.error= GEOJ_INCORRECT_GEOJSON; return TRUE; } @@ -2204,7 +2248,12 @@ bool Gis_multi_point::init_from_json(json_engine_t *je, bool er_on_3D, while (json_scan_next(je) == 0 && je->state != JST_ARRAY_END) { - DBUG_ASSERT(je->state == JST_VALUE); + if (je->state != JST_VALUE) + goto err_geoj_incorrect; + if (json_read_value(je)) + return TRUE; + if (je->value_type != JSON_VALUE_ARRAY) + goto err_geoj_incorrect; if (wkb->reserve(1 + 4, 512)) return TRUE; @@ -2551,11 +2600,9 @@ bool Gis_multi_line_string::init_from_json(json_engine_t *je, bool er_on_3D, uint32 n_line_strings= 0; uint32 ls_pos= wkb->length(); - if (json_read_value(je)) - return TRUE; - if (je->value_type != JSON_VALUE_ARRAY) { +err_geoj_incorrect: je->s.error= GEOJ_INCORRECT_GEOJSON; return TRUE; } @@ -2566,9 +2613,14 @@ bool Gis_multi_line_string::init_from_json(json_engine_t *je, bool er_on_3D, while (json_scan_next(je) == 0 && je->state != JST_ARRAY_END) { - Gis_line_string ls; - DBUG_ASSERT(je->state == JST_VALUE); + if (je->state != JST_VALUE) + goto err_geoj_incorrect; + if (json_read_value(je)) + return TRUE; + if (je->value_type != JSON_VALUE_ARRAY) + goto err_geoj_incorrect; + Gis_line_string ls; if (wkb->reserve(1 + 4, 512)) return TRUE; wkb->q_append((char) wkb_ndr); @@ -2953,11 +3005,9 @@ bool Gis_multi_polygon::init_from_json(json_engine_t *je, bool er_on_3D, int np_pos= wkb->length(); Gis_polygon p; - if (json_read_value(je)) - return TRUE; - if (je->value_type != JSON_VALUE_ARRAY) { +err_geoj_incorrect: je->s.error= GEOJ_INCORRECT_GEOJSON; return TRUE; } @@ -2968,7 +3018,12 @@ bool Gis_multi_polygon::init_from_json(json_engine_t *je, bool er_on_3D, while (json_scan_next(je) == 0 && je->state != JST_ARRAY_END) { - DBUG_ASSERT(je->state == JST_VALUE); + if (je->state != JST_VALUE) + goto err_geoj_incorrect; + if (json_read_value(je)) + return TRUE; + if (je->value_type != JSON_VALUE_ARRAY) + goto err_geoj_incorrect; if (wkb->reserve(1 + 4, 512)) return TRUE; @@ -3448,11 +3503,9 @@ bool Gis_geometry_collection::init_from_json(json_engine_t *je, bool er_on_3D, Geometry_buffer buffer; Geometry *g; - if (json_read_value(je)) - return TRUE; - if (je->value_type != JSON_VALUE_ARRAY) { +err_geoj_incorrect: je->s.error= GEOJ_INCORRECT_GEOJSON; return TRUE; } @@ -3463,17 +3516,17 @@ bool Gis_geometry_collection::init_from_json(json_engine_t *je, bool er_on_3D, while (json_scan_next(je) == 0 && je->state != JST_ARRAY_END) { - json_engine_t sav_je= *je; - - DBUG_ASSERT(je->state == JST_VALUE); + if (je->state == JST_VALUE) + { + if (json_read_value(je)) + return TRUE; + } + if (je->state != JST_OBJ_START) + goto err_geoj_incorrect; if (!(g= create_from_json(&buffer, je, er_on_3D, wkb))) return TRUE; - *je= sav_je; - if (json_skip_array_item(je)) - return TRUE; - n_objects++; } From bcd53fa3dfb9f0d058c2b865b214227b121d5947 Mon Sep 17 00:00:00 2001 From: Akshat Nehra Date: Wed, 29 Jul 2026 12:37:29 +1000 Subject: [PATCH 2/2] MDEV-39981 Fix ST_GEOMFROMGEOJSON returning wrong result with reversed key order ST_GEOMFROMGEOJSON requires JSON keys to appear in a specific order (type before geometries/features/geometry/coordinates). When keys appear in a different order, the JSON scanner is left in an inconsistent state because json_skip_level() is not called to advance past the value when the type has not yet been determined. Fix: add json_skip_level() calls in the geometries, features, and geometry key handlers when the type key has not yet been encountered. This mirrors the existing pattern in the coordinates handler. All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc. --- mysql-test/main/gis-json.result | 51 +++++++++++++++++++++++++++++++-- mysql-test/main/gis-json.test | 37 ++++++++++++++++++++++++ sql/spatial.cc | 8 ++++++ 3 files changed, 93 insertions(+), 3 deletions(-) diff --git a/mysql-test/main/gis-json.result b/mysql-test/main/gis-json.result index e636291b5db95..7843276698198 100644 --- a/mysql-test/main/gis-json.result +++ b/mysql-test/main/gis-json.result @@ -60,9 +60,7 @@ exp POINT(102 0.5) SELECT st_astext(st_geomfromgeojson('{ "geometry": { "type": "Point", "coordinates": [102.0, 0.5] }, "type": "Feature" }')) as exp; exp -NULL -Warnings: -Warning 4048 Incorrect GeoJSON format specified for st_geomfromgeojson function. +POINT(102 0.5) SELECT st_astext(st_geomfromgeojson('{ "type": "FeatureCollection", "features": [{ "type": "Feature", "geometry": { "type": "Point", "coordinates": [102.0, 0.5] }, "properties": { "prop0": "value0" } }]}')) as exp; exp GEOMETRYCOLLECTION(POINT(102 0.5)) @@ -212,4 +210,51 @@ SET @doc := CONCAT( SELECT ST_AsText(ST_GeomFromGeoJSON(@doc)) AS expr; expr GEOMETRYCOLLECTION(POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2),POINT(1 2)) +# +# MDEV-39981: ST_GEOMFROMGEOJSON returns wrong result with reversed key order +# +# GeometryCollection: type before geometries (baseline) +SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"type":"GeometryCollection","geometries":[]}')) as expr; +expr +GEOMETRYCOLLECTION EMPTY +# GeometryCollection: geometries before type (empty) +SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"geometries":[],"type":"GeometryCollection"}')) as expr; +expr +GEOMETRYCOLLECTION EMPTY +# GeometryCollection: geometries before type (non-empty) +SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"geometries":[{"type":"Point","coordinates":[0,1]}],"type":"GeometryCollection"}')) as expr; +expr +GEOMETRYCOLLECTION(POINT(0 1)) +# GeometryCollection: multiple geometries before type +SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"geometries":[{"type":"Point","coordinates":[1,2]},{"type":"Point","coordinates":[3,4]}],"type":"GeometryCollection"}')) as expr; +expr +GEOMETRYCOLLECTION(POINT(1 2),POINT(3 4)) +# Point: coordinates before type +SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"coordinates":[0,1],"type":"Point"}')) as expr; +expr +POINT(0 1) +# LineString: coordinates before type +SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"coordinates":[[0,0],[1,1],[2,2]],"type":"LineString"}')) as expr; +expr +LINESTRING(0 0,1 1,2 2) +# Polygon: coordinates before type +SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"coordinates":[[[0,0],[1,0],[1,1],[0,1],[0,0]]],"type":"Polygon"}')) as expr; +expr +POLYGON((0 0,1 0,1 1,0 1,0 0)) +# FeatureCollection: features before type +SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"features":[{"geometry":{"type":"Point","coordinates":[5,6]},"type":"Feature","properties":{}}],"type":"FeatureCollection"}')) as expr; +expr +GEOMETRYCOLLECTION(POINT(5 6)) +# Feature: geometry before type +SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"geometry":{"coordinates":[7,8],"type":"Point"},"type":"Feature","properties":{}}')) as expr; +expr +POINT(7 8) +# Unknown keys interspersed (should be ignored) +SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"foo":"bar","geometries":[],"type":"GeometryCollection","extra":123}')) as expr; +expr +GEOMETRYCOLLECTION EMPTY +# Deeply nested geometries before type +SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"geometries":[{"geometries":[{"type":"Point","coordinates":[9,10]}],"type":"GeometryCollection"}],"type":"GeometryCollection"}')) as expr; +expr +GEOMETRYCOLLECTION(GEOMETRYCOLLECTION(POINT(9 10))) # End of 10.6 tests diff --git a/mysql-test/main/gis-json.test b/mysql-test/main/gis-json.test index 33f00ee414459..e1b14c06e5b73 100644 --- a/mysql-test/main/gis-json.test +++ b/mysql-test/main/gis-json.test @@ -147,4 +147,41 @@ SET @doc := CONCAT( SELECT ST_AsText(ST_GeomFromGeoJSON(@doc)) AS expr; +--echo # +--echo # MDEV-39981: ST_GEOMFROMGEOJSON returns wrong result with reversed key order +--echo # + +--echo # GeometryCollection: type before geometries (baseline) +SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"type":"GeometryCollection","geometries":[]}')) as expr; + +--echo # GeometryCollection: geometries before type (empty) +SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"geometries":[],"type":"GeometryCollection"}')) as expr; + +--echo # GeometryCollection: geometries before type (non-empty) +SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"geometries":[{"type":"Point","coordinates":[0,1]}],"type":"GeometryCollection"}')) as expr; + +--echo # GeometryCollection: multiple geometries before type +SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"geometries":[{"type":"Point","coordinates":[1,2]},{"type":"Point","coordinates":[3,4]}],"type":"GeometryCollection"}')) as expr; + +--echo # Point: coordinates before type +SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"coordinates":[0,1],"type":"Point"}')) as expr; + +--echo # LineString: coordinates before type +SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"coordinates":[[0,0],[1,1],[2,2]],"type":"LineString"}')) as expr; + +--echo # Polygon: coordinates before type +SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"coordinates":[[[0,0],[1,0],[1,1],[0,1],[0,0]]],"type":"Polygon"}')) as expr; + +--echo # FeatureCollection: features before type +SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"features":[{"geometry":{"type":"Point","coordinates":[5,6]},"type":"Feature","properties":{}}],"type":"FeatureCollection"}')) as expr; + +--echo # Feature: geometry before type +SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"geometry":{"coordinates":[7,8],"type":"Point"},"type":"Feature","properties":{}}')) as expr; + +--echo # Unknown keys interspersed (should be ignored) +SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"foo":"bar","geometries":[],"type":"GeometryCollection","extra":123}')) as expr; + +--echo # Deeply nested geometries before type +SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"geometries":[{"geometries":[{"type":"Point","coordinates":[9,10]}],"type":"GeometryCollection"}],"type":"GeometryCollection"}')) as expr; + --echo # End of 10.6 tests diff --git a/sql/spatial.cc b/sql/spatial.cc index ee808a000428e..74c470bb98cb5 100644 --- a/sql/spatial.cc +++ b/sql/spatial.cc @@ -628,6 +628,9 @@ Geometry *Geometry::create_from_json(Geometry_buffer *buffer, arg= T_GEOMETRIES; argje= *je; je_arg= &argje; + /* skip geometries for now and search for type */ + if (json_skip_level(je)) + goto err_return; } else break; /* geometries needs to be an array */ @@ -656,6 +659,9 @@ Geometry *Geometry::create_from_json(Geometry_buffer *buffer, arg= T_FEATURE; argje= *je; je_arg= &argje; + /* skip features for now and search for type */ + if (json_skip_level(je)) + goto err_return; } else break; /* feature collections needs to be an array */ @@ -677,6 +683,8 @@ Geometry *Geometry::create_from_json(Geometry_buffer *buffer, arg= T_GEOMETRY; argje= *je; je_arg= &argje; + if (json_skip_level(je)) + goto err_return; } else break; /* geometry needs to be an object */