check transform mesh shape in _get_transform_mesh - #31894
Conversation
49a662c to
02b78f3
Compare
|
The red Tests jobs were the test_mlab TestSpectral fixture errors from pytest 9.1.0, not from this change (test_image passed). That got sorted on main with the pytest unpin, so I rebased onto current main to pick it up and CI should be green now. |
|
gentle ping |
eeshsaxena
left a comment
There was a problem hiding this comment.
Reviewed by reading (didn't build the extension locally), and the fix looks correct and worthwhile - this is really a memory-safety guard, not just a nicer error.
Tracing it through: mesh_dims is {dims[0]*dims[1], 2}, i.e. (out_h*out_w, 2), and the input mesh is built at exactly that size. The result is then forwarded via params.transform_mesh = transform_mesh.data() and consumed by the resampler as out_h*out_w coordinate pairs. The only prior validation was ndim == 2, so a (non-affine) inverse transform that returns e.g. (1, 2) passes that check but leaves the resampler reading past the end of the buffer. The new shape(0) != mesh_dims[0] || shape(1) != mesh_dims[1] check rejects exactly that case, and since a genuine 2-D -> 2-D transform must map N points to N points, there's no valid transform this would wrongly reject.
The message and the test line up too: the C++ raises "...should have shape ({}, {}) not ({}, {})" and the test's match="mesh array should have shape" is a substring, with BadMeshTransform.transform returning (1, 2) for a 9x9 output (81 != 1) to trip the new branch specifically.
Two small, optional thoughts:
- Might be worth a one-line comment noting this guards against an out-of-bounds read downstream, since that's the real motivation.
- Both the
ndimand shape checks now format an error; not worth changing, just noting the shape check makes the failure mode much clearer than a segfault would have.
Nice catch on the buffer over-read.
|
Good summary, that matches the intent exactly. Added a one-line comment above the shape check noting it guards the out-of-bounds read in the resampler. Left the two error paths separate since the ndim failure can't report a meaningful shape, so merging them would only blur the messages. |
…894-on-v3.11.x Backport PR #31894 on branch v3.11.x (check transform mesh shape in _get_transform_mesh)
PR summary
_get_transform_meshbuilds an input mesh sized to the output image, hands it to the supplied transform'sinverted().transform(), and forwards whatever comes back to the resampler as a flat table ofout_h * out_wcoordinate pairs. The only check on the returned array isndim == 2. A non-affine transform whose inverse returns fewer rows than requested, or a trailing dimension other than 2, leaveslookup_distortionin_image_resample.hindexing past the end of that buffer.Before: an undersized mesh produces an out-of-bounds read during resampling, and with a large output array it segfaults. After: the row and column counts are checked against the requested size and a
RuntimeErroris raised, alongside the existingndimcheck directly above it. The tradeoff is two integer comparisons per non-affine resample; keeping the check next to the mesh construction lets the resampler keep treating the buffer as exactly sized.Minimal repro on the current tree:
AI Disclosure
PR checklist