Skip to content

Commit 11f7a93

Browse files
AA-TurnerAA-Turner
authored
gh-132983: Split _zstd_set_c_parameters (#133921)
1 parent 0d499c7 commit 11f7a93

File tree

6 files changed

+228
-157
lines changed

6 files changed

+228
-157
lines changed

‎Lib/compression/zstd/_zstdfile.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import io
22
from os import PathLike
3-
from _zstd import (ZstdCompressor, ZstdDecompressor, ZstdError,
4-
ZSTD_DStreamOutSize)
3+
from _zstd import ZstdCompressor, ZstdDecompressor, ZSTD_DStreamOutSize
54
from compression._common import _streams
65

76
__all__ = ('ZstdFile', 'open')

‎Lib/test/test_zstd.py

Lines changed: 105 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,10 @@
6464

6565
SUPPORT_MULTITHREADING = False
6666

67+
C_INT_MIN = -(2**31)
68+
C_INT_MAX = (2**31) - 1
69+
70+
6771
def setUpModule():
6872
global SUPPORT_MULTITHREADING
6973
SUPPORT_MULTITHREADING = CompressionParameter.nb_workers.bounds() != (0, 0)
@@ -195,14 +199,21 @@ def test_simple_compress_bad_args(self):
195199
self.assertRaises(TypeError, ZstdCompressor, zstd_dict=b"abcd1234")
196200
self.assertRaises(TypeError, ZstdCompressor, zstd_dict={1: 2, 3: 4})
197201

198-
with self.assertRaises(ValueError):
199-
ZstdCompressor(2**31)
200-
with self.assertRaises(ValueError):
201-
ZstdCompressor(options={2**31: 100})
202+
# valid range for compression level is [-(1<<17), 22]
203+
msg = r'illegal compression level {}; the valid range is \[-?\d+, -?\d+\]'
204+
with self.assertRaisesRegex(ValueError, msg.format(C_INT_MAX)):
205+
ZstdCompressor(C_INT_MAX)
206+
with self.assertRaisesRegex(ValueError, msg.format(C_INT_MIN)):
207+
ZstdCompressor(C_INT_MIN)
208+
msg = r'illegal compression level; the valid range is \[-?\d+, -?\d+\]'
209+
with self.assertRaisesRegex(ValueError, msg):
210+
ZstdCompressor(level=-(2**1000))
211+
with self.assertRaisesRegex(ValueError, msg):
212+
ZstdCompressor(level=2**1000)
202213

203-
with self.assertRaises(ZstdError):
214+
with self.assertRaises(ValueError):
204215
ZstdCompressor(options={CompressionParameter.window_log: 100})
205-
with self.assertRaises(ZstdError):
216+
with self.assertRaises(ValueError):
206217
ZstdCompressor(options={3333: 100})
207218

208219
# Method bad arguments
@@ -253,18 +264,32 @@ def test_compress_parameters(self):
253264
}
254265
ZstdCompressor(options=d)
255266

256-
# larger than signed int, ValueError
257267
d1 = d.copy()
258-
d1[CompressionParameter.ldm_bucket_size_log] = 2**31
259-
self.assertRaises(ValueError, ZstdCompressor, options=d1)
268+
# larger than signed int
269+
d1[CompressionParameter.ldm_bucket_size_log] = C_INT_MAX
270+
with self.assertRaises(ValueError):
271+
ZstdCompressor(options=d1)
272+
# smaller than signed int
273+
d1[CompressionParameter.ldm_bucket_size_log] = C_INT_MIN
274+
with self.assertRaises(ValueError):
275+
ZstdCompressor(options=d1)
260276

261-
# clamp compressionLevel
277+
# out of bounds compression level
262278
level_min, level_max = CompressionParameter.compression_level.bounds()
263-
compress(b'', level_max+1)
264-
compress(b'', level_min-1)
265-
266-
compress(b'', options={CompressionParameter.compression_level:level_max+1})
267-
compress(b'', options={CompressionParameter.compression_level:level_min-1})
279+
with self.assertRaises(ValueError):
280+
compress(b'', level_max+1)
281+
with self.assertRaises(ValueError):
282+
compress(b'', level_min-1)
283+
with self.assertRaises(ValueError):
284+
compress(b'', 2**1000)
285+
with self.assertRaises(ValueError):
286+
compress(b'', -(2**1000))
287+
with self.assertRaises(ValueError):
288+
compress(b'', options={
289+
CompressionParameter.compression_level: level_max+1})
290+
with self.assertRaises(ValueError):
291+
compress(b'', options={
292+
CompressionParameter.compression_level: level_min-1})
268293

269294
# zstd lib doesn't support MT compression
270295
if not SUPPORT_MULTITHREADING:
@@ -277,19 +302,19 @@ def test_compress_parameters(self):
277302

278303
# out of bounds error msg
279304
option = {CompressionParameter.window_log:100}
280-
with self.assertRaisesRegex(ZstdError,
281-
(r'Error when setting zstd compression parameter "window_log", '
282-
r'it should \d+ <= value <= \d+, provided value is 100\. '
283-
r'\((?:32|64)-bit build\)')):
305+
with self.assertRaisesRegex(
306+
ValueError,
307+
"compression parameter 'window_log' received an illegal value 100; "
308+
r'the valid range is \[-?\d+, -?\d+\]',
309+
):
284310
compress(b'', options=option)
285311

286312
def test_unknown_compression_parameter(self):
287313
KEY = 100001234
288314
option = {CompressionParameter.compression_level: 10,
289315
KEY: 200000000}
290-
pattern = (r'Invalid zstd compression parameter.*?'
291-
fr'"unknown parameter \(key {KEY}\)"')
292-
with self.assertRaisesRegex(ZstdError, pattern):
316+
pattern = rf"invalid compression parameter 'unknown parameter \(key {KEY}\)'"
317+
with self.assertRaisesRegex(ValueError, pattern):
293318
ZstdCompressor(options=option)
294319

295320
@unittest.skipIf(not SUPPORT_MULTITHREADING,
@@ -384,12 +409,22 @@ def test_simple_decompress_bad_args(self):
384409
self.assertRaises(TypeError, ZstdDecompressor, options=b'abc')
385410

386411
with self.assertRaises(ValueError):
387-
ZstdDecompressor(options={2**31 : 100})
412+
ZstdDecompressor(options={C_INT_MAX: 100})
413+
with self.assertRaises(ValueError):
414+
ZstdDecompressor(options={C_INT_MIN: 100})
415+
with self.assertRaises(ValueError):
416+
ZstdDecompressor(options={0: C_INT_MAX})
417+
with self.assertRaises(OverflowError):
418+
ZstdDecompressor(options={2**1000: 100})
419+
with self.assertRaises(OverflowError):
420+
ZstdDecompressor(options={-(2**1000): 100})
421+
with self.assertRaises(OverflowError):
422+
ZstdDecompressor(options={0: -(2**1000)})
388423

389-
with self.assertRaises(ZstdError):
390-
ZstdDecompressor(options={DecompressionParameter.window_log_max:100})
391-
with self.assertRaises(ZstdError):
392-
ZstdDecompressor(options={3333 : 100})
424+
with self.assertRaises(ValueError):
425+
ZstdDecompressor(options={DecompressionParameter.window_log_max: 100})
426+
with self.assertRaises(ValueError):
427+
ZstdDecompressor(options={3333: 100})
393428

394429
empty = compress(b'')
395430
lzd = ZstdDecompressor()
@@ -402,26 +437,52 @@ def test_decompress_parameters(self):
402437
d = {DecompressionParameter.window_log_max : 15}
403438
ZstdDecompressor(options=d)
404439

405-
# larger than signed int, ValueError
406440
d1 = d.copy()
407-
d1[DecompressionParameter.window_log_max] = 2**31
408-
self.assertRaises(ValueError, ZstdDecompressor, None, d1)
441+
# larger than signed int
442+
d1[DecompressionParameter.window_log_max] = 2**1000
443+
with self.assertRaises(OverflowError):
444+
ZstdDecompressor(None, d1)
445+
# smaller than signed int
446+
d1[DecompressionParameter.window_log_max] = -(2**1000)
447+
with self.assertRaises(OverflowError):
448+
ZstdDecompressor(None, d1)
449+
450+
d1[DecompressionParameter.window_log_max] = C_INT_MAX
451+
with self.assertRaises(ValueError):
452+
ZstdDecompressor(None, d1)
453+
d1[DecompressionParameter.window_log_max] = C_INT_MIN
454+
with self.assertRaises(ValueError):
455+
ZstdDecompressor(None, d1)
409456

410457
# out of bounds error msg
411458
options = {DecompressionParameter.window_log_max:100}
412-
with self.assertRaisesRegex(ZstdError,
413-
(r'Error when setting zstd decompression parameter "window_log_max", '
414-
r'it should \d+ <= value <= \d+, provided value is 100\. '
415-
r'\((?:32|64)-bit build\)')):
459+
with self.assertRaisesRegex(
460+
ValueError,
461+
"decompression parameter 'window_log_max' received an illegal value 100; "
462+
r'the valid range is \[-?\d+, -?\d+\]',
463+
):
464+
decompress(b'', options=options)
465+
466+
# out of bounds deecompression parameter
467+
options[DecompressionParameter.window_log_max] = C_INT_MAX
468+
with self.assertRaises(ValueError):
469+
decompress(b'', options=options)
470+
options[DecompressionParameter.window_log_max] = C_INT_MIN
471+
with self.assertRaises(ValueError):
472+
decompress(b'', options=options)
473+
options[DecompressionParameter.window_log_max] = 2**1000
474+
with self.assertRaises(OverflowError):
475+
decompress(b'', options=options)
476+
options[DecompressionParameter.window_log_max] = -(2**1000)
477+
with self.assertRaises(OverflowError):
416478
decompress(b'', options=options)
417479

418480
def test_unknown_decompression_parameter(self):
419481
KEY = 100001234
420482
options = {DecompressionParameter.window_log_max: DecompressionParameter.window_log_max.bounds()[1],
421483
KEY: 200000000}
422-
pattern = (r'Invalid zstd decompression parameter.*?'
423-
fr'"unknown parameter \(key {KEY}\)"')
424-
with self.assertRaisesRegex(ZstdError, pattern):
484+
pattern = rf"invalid decompression parameter 'unknown parameter \(key {KEY}\)'"
485+
with self.assertRaisesRegex(ValueError, pattern):
425486
ZstdDecompressor(options=options)
426487

427488
def test_decompress_epilogue_flags(self):
@@ -1424,11 +1485,11 @@ def test_init_bad_mode(self):
14241485
ZstdFile(io.BytesIO(COMPRESSED_100_PLUS_32KB), "rw")
14251486

14261487
with self.assertRaisesRegex(TypeError,
1427-
r"NOT be a CompressionParameter"):
1488+
r"not be a CompressionParameter"):
14281489
ZstdFile(io.BytesIO(), 'rb',
14291490
options={CompressionParameter.compression_level:5})
14301491
with self.assertRaisesRegex(TypeError,
1431-
r"NOT be a DecompressionParameter"):
1492+
r"not be a DecompressionParameter"):
14321493
ZstdFile(io.BytesIO(), 'wb',
14331494
options={DecompressionParameter.window_log_max:21})
14341495

@@ -1439,19 +1500,19 @@ def test_init_bad_check(self):
14391500
with self.assertRaises(TypeError):
14401501
ZstdFile(io.BytesIO(), "w", level='asd')
14411502
# CHECK_UNKNOWN and anything above CHECK_ID_MAX should be invalid.
1442-
with self.assertRaises(ZstdError):
1503+
with self.assertRaises(ValueError):
14431504
ZstdFile(io.BytesIO(), "w", options={999:9999})
1444-
with self.assertRaises(ZstdError):
1505+
with self.assertRaises(ValueError):
14451506
ZstdFile(io.BytesIO(), "w", options={CompressionParameter.window_log:99})
14461507

14471508
with self.assertRaises(TypeError):
14481509
ZstdFile(io.BytesIO(COMPRESSED_100_PLUS_32KB), "r", options=33)
14491510

1450-
with self.assertRaises(ValueError):
1511+
with self.assertRaises(OverflowError):
14511512
ZstdFile(io.BytesIO(COMPRESSED_100_PLUS_32KB),
14521513
options={DecompressionParameter.window_log_max:2**31})
14531514

1454-
with self.assertRaises(ZstdError):
1515+
with self.assertRaises(ValueError):
14551516
ZstdFile(io.BytesIO(COMPRESSED_100_PLUS_32KB),
14561517
options={444:333})
14571518

@@ -1467,7 +1528,7 @@ def test_init_close_fp(self):
14671528
tmp_f.write(DAT_130K_C)
14681529
filename = tmp_f.name
14691530

1470-
with self.assertRaises(ValueError):
1531+
with self.assertRaises(TypeError):
14711532
ZstdFile(filename, options={'a':'b'})
14721533

14731534
# for PyPy

‎Modules/_zstd/_zstdmodule.c

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -103,16 +103,13 @@ static const ParameterInfo dp_list[] = {
103103
};
104104

105105
void
106-
set_parameter_error(const _zstd_state* const state, int is_compress,
107-
int key_v, int value_v)
106+
set_parameter_error(int is_compress, int key_v, int value_v)
108107
{
109108
ParameterInfo const *list;
110109
int list_size;
111-
char const *name;
112110
char *type;
113111
ZSTD_bounds bounds;
114-
int i;
115-
char pos_msg[128];
112+
char pos_msg[64];
116113

117114
if (is_compress) {
118115
list = cp_list;
@@ -126,8 +123,8 @@ set_parameter_error(const _zstd_state* const state, int is_compress,
126123
}
127124

128125
/* Find parameter's name */
129-
name = NULL;
130-
for (i = 0; i < list_size; i++) {
126+
char const *name = NULL;
127+
for (int i = 0; i < list_size; i++) {
131128
if (key_v == (list+i)->parameter) {
132129
name = (list+i)->parameter_name;
133130
break;
@@ -149,20 +146,16 @@ set_parameter_error(const _zstd_state* const state, int is_compress,
149146
bounds = ZSTD_dParam_getBounds(key_v);
150147
}
151148
if (ZSTD_isError(bounds.error)) {
152-
PyErr_Format(state->ZstdError,
153-
"Invalid zstd %s parameter \"%s\".",
149+
PyErr_Format(PyExc_ValueError, "invalid %s parameter '%s'",
154150
type, name);
155151
return;
156152
}
157153

158154
/* Error message */
159-
PyErr_Format(state->ZstdError,
160-
"Error when setting zstd %s parameter \"%s\", it "
161-
"should %d <= value <= %d, provided value is %d. "
162-
"(%d-bit build)",
163-
type, name,
164-
bounds.lowerBound, bounds.upperBound, value_v,
165-
8*(int)sizeof(Py_ssize_t));
155+
PyErr_Format(PyExc_ValueError,
156+
"%s parameter '%s' received an illegal value %d; "
157+
"the valid range is [%d, %d]",
158+
type, name, value_v, bounds.lowerBound, bounds.upperBound);
166159
}
167160

168161
static inline _zstd_state*

‎Modules/_zstd/_zstdmodule.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ set_zstd_error(const _zstd_state* const state,
4949
const error_type type, size_t zstd_ret);
5050

5151
extern void
52-
set_parameter_error(const _zstd_state* const state, int is_compress,
53-
int key_v, int value_v);
52+
set_parameter_error(int is_compress, int key_v, int value_v);
5453

5554
#endif // !ZSTD_MODULE_H

0 commit comments

Comments
 (0)