MDEV-39813 ST_GeomFromGeoJSON does not control recursion depth - #5379
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request re-enables the gis-json test and addresses a potential infinite recursion issue in Geometry::create_from_json by adding a stack overrun check using check_stack_overrun. Unused variables related to the JSON engine's stack pointer were also cleaned up. The review feedback recommends refactoring Geometry::create_from_json to accept THD *thd as a parameter rather than calling current_thd internally, which avoids costly thread-local lookups during recursive execution.
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.
gkodinov
added
the
MariaDB Foundation
f5d9eb0 to
444de02
Compare
e48a204 to
c8b1ea8
Compare
bbfea7a to
2dbaba2
Compare
|
It looks like windows fail connected to the fix, could you please check? |
Using stack_p wasn't a portable concept in 12.3 when JSON parsing got unlimited depth. To let ST_GeomFromGeoJSON was already a recursive function, needed because object order of "type" may be after the "geometries", but with json_engine_t no longer enforcing the depth, some stack checking was required. Use the check_stack_depth function to allow excessively deep GeoJSON objects to error. As this is cleaned up the gis-json test can be enabled. The exceeding stack depth is moved to lotofstack.test.
2dbaba2 to
d226001
Compare
changed to lotofstack based on 9a84c86 |
4 participants
Using stack_p wasn't a portable concept in 12.3 when JSON parsing got unlimited depth. To let ST_GeomFromGeoJSON as a recursive function, needed because object order of "type" may be after the "geometries", some stack checking was required.
Use the check_stack_depth function to allow excessively deep GeoJSON objects to error.
@mariadb-RuchaDeodhar , I'm after a concept approval. To run reliably I'm going to move this test to the thread_stack_overun.test that @vaintroub added in 10.11 (#5320) that hasn't yet been merged up.
The 10.6/10.11 fix for this bug is going to change the stack_p, killed_ptr and json_scan_start but what is clear is that 12.3 requires a stack overrun check.