MDEV-39981 Fix ST_GEOMFROMGEOJSON returning wrong result with reverse… by akshatnehra · Pull Request #5243 · MariaDB/server

akshatnehra · 2026-06-15T17:32:41Z

Description

This PR fixes MDEV-39981 where ST_GEOMFROMGEOJSON() crashes or returns wrong results when GeoJSON keys appear in a non-standard order (e.g., "geometries" before "type").

The GeoJSON parser in spatial.cc expects "type" to appear before other keys. When "geometries", "features", or "geometry" appear first, the parser saves the value start position for later re-parsing but fails to call json_skip_level() to advance the scanner past the value. This leaves the JSON scanner in an inconsistent state, causing crashes or incorrect parsing of subsequent keys.

The fix involves:

Adding json_skip_level() calls in the three affected key handlers (geometries, features, geometry) when the type has not yet been determined — matching the existing pattern already used by the coordinates handler.

How can this PR be tested?

Run the MTR test:

build/mysql-test/mtr main.mdev_39981

Or manually (crashes without fix):

SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"geometries":[],"type":"GeometryCollection"}'));
SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"features":[{"type":"Feature","geometry":{"type":"Point","coordinates":[5,6]},"properties":{}}],"type":"FeatureCollection"}'));
SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"geometry":{"type":"Point","coordinates":[7,8]},"type":"Feature","properties":{}}'));

Results from my testing

Before changes (server crashes)

MariaDB> SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"geometries":[],"type":"GeometryCollection"}'));
ERROR 2026 (HY000): TLS/SSL error: unexpected eof while reading
-- (server crashed)

After changes

MariaDB> SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"geometries":[],"type":"GeometryCollection"}')) AS r1;
+------------------------+
| r1                     |
+------------------------+
| GEOMETRYCOLLECTION EMPTY|
+------------------------+

MariaDB> SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"features":[...],"type":"FeatureCollection"}')) AS r2;
+------------------------------+
| r2                           |
+------------------------------+
| GEOMETRYCOLLECTION(POINT(5 6))|
+------------------------------+

MariaDB> SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"geometry":{"type":"Point","coordinates":[7,8]},"type":"Feature","properties":{}}')) AS r3;
+------------+
| r3         |
+------------+
| POINT(7 8) |
+------------+

GeoJSON is now parsed correctly regardless of key order, as required by the RFC 7946 spec (key order MUST NOT affect interpretation).

Basing the PR against the correct MariaDB version

This is a bug fix and the PR is based against the latest MariaDB development branch.

PR quality check

I have checked the CODING_STANDARDS.md file and my PR conforms to this where appropriate.
For any trivial modifications to the PR, I am fine with the reviewer making the changes themselves.

…d 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.

gemini-code-assist

Code Review

This pull request addresses MDEV-39981, where ST_GEOMFROMGEOJSON returns incorrect results when GeoJSON keys are in a reversed order (such as coordinates or geometries appearing before the type). The fix introduces calls to json_skip_level in sql/spatial.cc to properly skip over nested structures when searching for the type key. Comprehensive test cases have been added to verify various geometries and key orderings. No review comments were provided, and there is no additional feedback to offer.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

grooverdan

Thanks very much for writing this. I was currently working on MDEV-39813 and was seeing this gap in implementation.

As a bug fix, can you rebase base to 10.11. There is a chance I could take it to 106 if needed to complete MDEV-39813, but I'm reviewing this to ensure changes don't conflict.

If you can take my suggested fixes on for MDEV-39989 and put them on the same branch it would ensure they don't conflict or merge incorrectly. One more MDEV-39989 fix is in coordinates for type !== JSON_VALUE_ARRAY, break;.

This only fixes part of the geometries processing MDEV-39989 invalid calls, however the MDEV-39813 will replace the json_scan_start additional calls, and might correct the processing of other GIS types.

grooverdan · 2026-06-16T05:33:41Z

@@ -0,0 +1,38 @@
+--echo #
+--echo # MDEV-39981: ST_GEOMFROMGEOJSON returns wrong result with reversed key order


Can these tests be moved to the bottom of mysql-test/main/gis-json.test

grooverdan · 2026-06-16T05:35:01Z

+--echo #
+
+--echo # GeometryCollection: type before geometries (baseline)
+SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"type":"GeometryCollection","geometries":[]}'));


If you run this test with mtr --view-protocol it will fail. The solution is to use SELECT ST_ASTEXT(......) as exp as a pattern to give every result a simple name. Same with all other queries.

Thanks for the incremental headers as separation.

grooverdan · 2026-06-16T05:39:06Z

+SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"foo":"bar","geometries":[],"type":"GeometryCollection","extra":123}'));
+
+--echo # Deeply nested geometries before type
+SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"geometries":[{"type":"GeometryCollection","geometries":[{"type":"Point","coordinates":[9,10]}]}],"type":"GeometryCollection"}'));


could re-order the "type: Point" and its "coordinates" within the nested version.

grooverdan · 2026-06-16T05:39:45Z

+
+--echo # Deeply nested geometries before type
+SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"geometries":[{"type":"GeometryCollection","geometries":[{"type":"Point","coordinates":[9,10]}]}],"type":"GeometryCollection"}'));
+


Below would be `--echo # End of 10.11 tests" once moved.

grooverdan · 2026-06-16T06:37:16Z

          goto create_geom;
        }
+        if (json_skip_level(je))
+          goto err_return;


just below the below } we could append else break; which would catch where geometries is something other than an array. It could be considered part of MDEV-39989.

grooverdan · 2026-06-16T06:38:01Z

        if (fcoll_type_found)
          goto handle_feature_collection;
+        if (json_skip_level(je))
+          goto err_return;


like above, features can only be an array. Same else break (!= JSON_VALUE_ARRAY) to report error.

grooverdan · 2026-06-16T06:53:50Z

        if (feature_type_found)
          goto handle_geometry_key;
+        if (json_skip_level(je))
+          goto err_return;


Geometry is a object type only. Like above, can result in error with else break; condition

grooverdan · 2026-06-16T06:58:41Z

+SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"coordinates":[[[0,0],[1,0],[1,1],[0,1],[0,0]]],"type":"Polygon"}'));
+
+--echo # FeatureCollection: features before type
+SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"features":[{"type":"Feature","geometry":{"type":"Point","coordinates":[5,6]},"properties":{}}],"type":"FeatureCollection"}'));


could put type: Feature after geometry and type: point after properties. I expect the same, but it adds to the completeness of type last processing.

gkodinov

Thank you for your contribution!

Daniel already is doing a great job in reviewing this, so I'll just restrict myself to the formalities: please rebase on 10.11 (as he has already suggested). And also format the test addition properly as suggested by him.

gemini-code-assist Bot reviewed Jun 15, 2026

View reviewed changes

gkodinov added the External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements. label Jun 16, 2026

grooverdan requested changes Jun 16, 2026

View reviewed changes

grooverdan self-assigned this Jun 16, 2026

gkodinov requested changes Jun 16, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

MDEV-39981 Fix ST_GEOMFROMGEOJSON returning wrong result with reverse…#5243

MDEV-39981 Fix ST_GEOMFROMGEOJSON returning wrong result with reverse…#5243
akshatnehra wants to merge 1 commit into
MariaDB:mainfrom
akshatnehra:MDEV-39981

akshatnehra commented Jun 15, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

grooverdan left a comment

Uh oh!

grooverdan Jun 16, 2026

Uh oh!

grooverdan Jun 16, 2026 •

edited

Loading

Uh oh!

grooverdan Jun 16, 2026

Uh oh!

grooverdan Jun 16, 2026

Uh oh!

grooverdan Jun 16, 2026

Uh oh!

grooverdan Jun 16, 2026

Uh oh!

grooverdan Jun 16, 2026

Uh oh!

grooverdan Jun 16, 2026

Uh oh!

gkodinov left a comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

		@@ -0,0 +1,38 @@
		--echo #
		--echo # MDEV-39981: ST_GEOMFROMGEOJSON returns wrong result with reversed key order


		--echo # Deeply nested geometries before type
		SELECT ST_ASTEXT(ST_GEOMFROMGEOJSON('{"geometries":[{"type":"GeometryCollection","geometries":[{"type":"Point","coordinates":[9,10]}]}],"type":"GeometryCollection"}'));

Uh oh!

Conversation

akshatnehra commented Jun 15, 2026

Description

How can this PR be tested?

Results from my testing

Basing the PR against the correct MariaDB version

PR quality check

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

grooverdan left a comment

Choose a reason for hiding this comment

Uh oh!

grooverdan Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

grooverdan Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

grooverdan Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

grooverdan Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

grooverdan Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

grooverdan Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

grooverdan Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

grooverdan Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

gkodinov left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

grooverdan Jun 16, 2026 •

edited

Loading