Skip to content

Commit 999725f

Browse files
rouaultrouault
authored
Merge pull request OSGeo#12818 from rouault/GDALNoDataMaskBand_corruption
GDALNoDataMaskBand::IRasterIO(): fix corruption when reading from Byte band and nLineSpace > nBufXSize (3.10.0 regression)
2 parents 874bd25 + 8339e9a commit 999725f

File tree

2 files changed

+142
-95
lines changed

2 files changed

+142
-95
lines changed

‎autotest/gcore/nodatamaskband.py‎

Lines changed: 106 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -14,109 +14,133 @@
1414

1515
import struct
1616

17-
from osgeo import gdal
17+
import pytest
1818

19+
from osgeo import gdal
1920

20-
def test_nodatamaskband_1():
2121

22-
for (dt, struct_type, v) in [
22+
@pytest.mark.parametrize(
23+
"dt,struct_type,v",
24+
[
2325
(gdal.GDT_Byte, "B", 255),
26+
(gdal.GDT_Int8, "b", 127),
2427
(gdal.GDT_Int16, "h", 32767),
2528
(gdal.GDT_UInt16, "H", 65535),
2629
(gdal.GDT_Int32, "i", 0x7FFFFFFF),
2730
(gdal.GDT_UInt32, "I", 0xFFFFFFFF),
31+
(gdal.GDT_Int64, "q", 0x7FFFFFFFFFFFFFFF),
32+
(gdal.GDT_UInt64, "Q", 0xFFFFFFFFFFFFFFFF),
33+
(gdal.GDT_Float16, "e", 1.25),
2834
(gdal.GDT_Float32, "f", 1.25),
2935
(gdal.GDT_Float32, "f", float("nan")),
3036
(gdal.GDT_Float64, "d", 1.2345678),
3137
(gdal.GDT_Float64, "d", float("nan")),
32-
]:
33-
34-
ds = gdal.GetDriverByName("MEM").Create("", 6, 4, 1, dt)
35-
ds.GetRasterBand(1).SetNoDataValue(v)
36-
ds.GetRasterBand(1).WriteRaster(
37-
0,
38-
0,
39-
6,
40-
4,
41-
struct.pack(
42-
struct_type * 6 * 4,
43-
v,
44-
1,
45-
1,
46-
v,
47-
v,
48-
0,
49-
v,
50-
1,
51-
1,
52-
v,
53-
v,
54-
0,
55-
v,
56-
1,
57-
1,
58-
v,
59-
v,
60-
0,
61-
0,
62-
v,
63-
v,
64-
0,
65-
0,
66-
0,
67-
),
68-
)
38+
],
39+
)
40+
def test_nodatamaskband_1(dt, struct_type, v):
6941

70-
data = ds.GetRasterBand(1).GetMaskBand().ReadRaster()
71-
data_ar = struct.unpack("B" * 6 * 4, data)
72-
expected_ar = (
73-
0,
74-
255,
75-
255,
42+
ds = gdal.GetDriverByName("MEM").Create("", 6, 4, 1, dt)
43+
ds.GetRasterBand(1).SetNoDataValue(v)
44+
ds.GetRasterBand(1).WriteRaster(
45+
0,
46+
0,
47+
6,
48+
4,
49+
struct.pack(
50+
struct_type * 6 * 4,
51+
v,
52+
1,
53+
1,
54+
v,
55+
v,
7656
0,
57+
v,
58+
1,
59+
1,
60+
v,
61+
v,
7762
0,
78-
255,
63+
v,
64+
1,
65+
1,
66+
v,
67+
v,
7968
0,
80-
255,
81-
255,
8269
0,
70+
v,
71+
v,
8372
0,
84-
255,
8573
0,
86-
255,
87-
255,
8874
0,
89-
0,
90-
255,
91-
255,
92-
0,
93-
0,
94-
255,
95-
255,
96-
255,
97-
)
98-
assert data_ar == expected_ar, dt
75+
),
76+
)
77+
78+
data = ds.GetRasterBand(1).GetMaskBand().ReadRaster()
79+
data_ar = struct.unpack("B" * 6 * 4, data)
80+
expected_ar = (
81+
0,
82+
255,
83+
255,
84+
0,
85+
0,
86+
255,
87+
0,
88+
255,
89+
255,
90+
0,
91+
0,
92+
255,
93+
0,
94+
255,
95+
255,
96+
0,
97+
0,
98+
255,
99+
255,
100+
0,
101+
0,
102+
255,
103+
255,
104+
255,
105+
)
106+
assert data_ar == expected_ar, dt
107+
108+
data = ds.GetRasterBand(1).GetMaskBand().ReadRaster(buf_line_space=8)
109+
data_ar = struct.unpack("B" * (8 * 3 + 6), data)
110+
for y in range(4):
111+
for x in range(6):
112+
assert data_ar[y * 8 + x] == expected_ar[y * 6 + x]
113+
114+
data = (
115+
ds.GetRasterBand(1)
116+
.GetMaskBand()
117+
.ReadRaster(buf_pixel_space=2, buf_line_space=2 * 6)
118+
)
119+
data_ar = struct.unpack("B" * ((6 * 3 + 5) * 2 + 1), data)
120+
for y in range(4):
121+
for x in range(6):
122+
assert data_ar[(y * 6 + x) * 2] == expected_ar[y * 6 + x]
99123

100-
data = ds.GetRasterBand(1).GetMaskBand().ReadBlock(0, 0)
101-
data_ar = struct.unpack("B" * 6 * 1, data)
102-
expected_ar = (0, 255, 255, 0, 0, 255)
103-
assert data_ar == expected_ar, dt
124+
data = ds.GetRasterBand(1).GetMaskBand().ReadBlock(0, 0)
125+
data_ar = struct.unpack("B" * 6 * 1, data)
126+
expected_ar = (0, 255, 255, 0, 0, 255)
127+
assert data_ar == expected_ar, dt
104128

105-
data = ds.GetRasterBand(1).GetMaskBand().ReadBlock(0, 3)
106-
data_ar = struct.unpack("B" * 6 * 1, data)
107-
expected_ar = (255, 0, 0, 255, 255, 255)
108-
assert data_ar == expected_ar, dt
129+
data = ds.GetRasterBand(1).GetMaskBand().ReadBlock(0, 3)
130+
data_ar = struct.unpack("B" * 6 * 1, data)
131+
expected_ar = (255, 0, 0, 255, 255, 255)
132+
assert data_ar == expected_ar, dt
109133

110-
data = ds.GetRasterBand(1).GetMaskBand().ReadRaster(buf_xsize=3, buf_ysize=2)
111-
data_ar = struct.unpack("B" * 3 * 2, data)
112-
expected_ar = (255, 0, 255, 0, 255, 255)
113-
assert data_ar == expected_ar, dt
134+
data = ds.GetRasterBand(1).GetMaskBand().ReadRaster(buf_xsize=3, buf_ysize=2)
135+
data_ar = struct.unpack("B" * 3 * 2, data)
136+
expected_ar = (255, 0, 255, 0, 255, 255)
137+
assert data_ar == expected_ar, dt
114138

115-
data = (
116-
ds.GetRasterBand(1)
117-
.GetMaskBand()
118-
.ReadRaster(buf_type=gdal.GDT_UInt16, buf_xsize=3, buf_ysize=2)
119-
)
120-
data_ar = struct.unpack("H" * 3 * 2, data)
121-
expected_ar = (255, 0, 255, 0, 255, 255)
122-
assert data_ar == expected_ar, dt
139+
data = (
140+
ds.GetRasterBand(1)
141+
.GetMaskBand()
142+
.ReadRaster(buf_type=gdal.GDT_UInt16, buf_xsize=3, buf_ysize=2)
143+
)
144+
data_ar = struct.unpack("H" * 3 * 2, data)
145+
expected_ar = (255, 0, 255, 0, 255, 255)
146+
assert data_ar == expected_ar, dt

‎gcore/gdalnodatamaskband.cpp‎

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include "gdal_priv.h"
1717

1818
#include <algorithm>
19+
#include <cassert>
1920
#include <cmath>
2021
#include <cstring>
2122
#include <utility>
@@ -157,6 +158,11 @@ bool GDALNoDataMaskBand::IsNoDataInRange(double dfNoDataValue,
157158
return GDALIsValueInRange<GByte>(dfNoDataValue);
158159
}
159160

161+
case GDT_Int8:
162+
{
163+
return GDALIsValueInRange<signed char>(dfNoDataValue);
164+
}
165+
160166
case GDT_Int16:
161167
{
162168
return GDALIsValueInRange<GInt16>(dfNoDataValue);
@@ -203,10 +209,18 @@ bool GDALNoDataMaskBand::IsNoDataInRange(double dfNoDataValue,
203209
return true;
204210
}
205211

206-
default:
207-
CPLAssert(false);
208-
return false;
212+
case GDT_CFloat16:
213+
case GDT_CFloat32:
214+
case GDT_CFloat64:
215+
case GDT_CInt16:
216+
case GDT_CInt32:
217+
case GDT_Unknown:
218+
case GDT_TypeCount:
219+
break;
209220
}
221+
222+
CPLAssert(false);
223+
return false;
210224
}
211225

212226
/************************************************************************/
@@ -268,12 +282,7 @@ static void SetZeroOr255(GByte *pabyDest, const T *panSrc, int nBufXSize,
268282
int nBufYSize, GSpacing nPixelSpace,
269283
GSpacing nLineSpace, T nNoData)
270284
{
271-
if (nPixelSpace == 1 && nLineSpace == nBufXSize)
272-
{
273-
const size_t nBufSize = static_cast<size_t>(nBufXSize) * nBufYSize;
274-
SetZeroOr255(pabyDest, panSrc, nBufSize, nNoData);
275-
}
276-
else if (nPixelSpace == 1)
285+
if (nPixelSpace == 1)
277286
{
278287
for (int iY = 0; iY < nBufYSize; iY++)
279288
{
@@ -319,7 +328,8 @@ CPLErr GDALNoDataMaskBand::IRasterIO(GDALRWFlag eRWFlag, int nXOff, int nYOff,
319328
// Optimization in common use case (#4488).
320329
// This avoids triggering the block cache on this band, which helps
321330
// reducing the global block cache consumption.
322-
if (eBufType == GDT_Byte && eWrkDT == GDT_Byte)
331+
if (eBufType == GDT_Byte && eWrkDT == GDT_Byte && nPixelSpace == 1 &&
332+
nLineSpace >= nBufXSize)
323333
{
324334
const CPLErr eErr = m_poParent->RasterIO(
325335
GF_Read, nXOff, nYOff, nXSize, nYSize, pData, nBufXSize, nBufYSize,
@@ -330,15 +340,19 @@ CPLErr GDALNoDataMaskBand::IRasterIO(GDALRWFlag eRWFlag, int nXOff, int nYOff,
330340
GByte *pabyData = static_cast<GByte *>(pData);
331341
const GByte byNoData = static_cast<GByte>(m_dfNoDataValue);
332342

333-
if (nPixelSpace == 1 && nLineSpace == nBufXSize)
343+
if (nLineSpace == nBufXSize)
334344
{
335345
const size_t nBufSize = static_cast<size_t>(nBufXSize) * nBufYSize;
336346
SetZeroOr255(pabyData, nBufSize, byNoData);
337347
}
338348
else
339349
{
340-
SetZeroOr255(pabyData, pabyData, nBufXSize, nBufYSize, nPixelSpace,
341-
nLineSpace, byNoData);
350+
assert(nLineSpace > nBufXSize);
351+
for (int iY = 0; iY < nBufYSize; iY++)
352+
{
353+
SetZeroOr255(pabyData, nBufXSize, byNoData);
354+
pabyData += nLineSpace;
355+
}
342356
}
343357
return CE_None;
344358
}
@@ -421,6 +435,15 @@ CPLErr GDALNoDataMaskBand::IRasterIO(GDALRWFlag eRWFlag, int nXOff, int nYOff,
421435
*/
422436
switch (eWrkDT)
423437
{
438+
case GDT_Byte:
439+
{
440+
const auto nNoData = static_cast<GByte>(m_dfNoDataValue);
441+
const auto *panSrc = static_cast<const GByte *>(pTemp);
442+
SetZeroOr255(pabyDest, panSrc, nBufXSize, nBufYSize,
443+
nPixelSpace, nLineSpace, nNoData);
444+
}
445+
break;
446+
424447
case GDT_Int16:
425448
{
426449
const auto nNoData = static_cast<int16_t>(m_dfNoDataValue);

0 commit comments

Comments
 (0)