Skip to content

Commit 5b851a2

Browse files
AA-TurnerAA-Turner
committed
Serhiy's review
1 parent 86860c8 commit 5b851a2

File tree

3 files changed

+66
-47
lines changed

3 files changed

+66
-47
lines changed

‎Lib/test/test_zstd.py‎

Lines changed: 48 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -196,14 +196,31 @@ def test_simple_compress_bad_args(self):
196196
self.assertRaises(TypeError, ZstdCompressor, zstd_dict={1: 2, 3: 4})
197197

198198
# valid compression level range is [-(1<<17), 22]
199-
with self.assertRaises(ValueError):
199+
with self.assertRaises(ValueError) as cm:
200200
ZstdCompressor(23)
201-
with self.assertRaises(ValueError):
201+
self.assertEqual(
202+
str(cm.exception),
203+
'23 not in valid range -131072 <= compression level <= 22.',
204+
)
205+
with self.assertRaises(ValueError) as cm:
202206
ZstdCompressor(-(1<<17)-1)
203-
with self.assertRaises(ValueError):
207+
self.assertEqual(-(1<<17)-1, -131073)
208+
self.assertEqual(
209+
str(cm.exception),
210+
'-131073 not in valid range -131072 <= compression level <= 22.',
211+
)
212+
with self.assertRaises(ValueError) as cm:
204213
ZstdCompressor(2**31)
205-
with self.assertRaises(ValueError):
214+
self.assertEqual(
215+
str(cm.exception),
216+
'compression level not in valid range -131072 <= level <= 22.',
217+
)
218+
with self.assertRaises(ValueError) as cm:
206219
ZstdCompressor(level=-(2**1000))
220+
self.assertEqual(
221+
str(cm.exception),
222+
'compression level not in valid range -131072 <= level <= 22.',
223+
)
207224
with self.assertRaises(ValueError):
208225
ZstdCompressor(level=(2**1000))
209226

@@ -260,10 +277,15 @@ def test_compress_parameters(self):
260277
}
261278
ZstdCompressor(options=d)
262279

263-
# larger than signed int, ValueError
264280
d1 = d.copy()
281+
# larger than signed int
265282
d1[CompressionParameter.ldm_bucket_size_log] = 2**31
266-
self.assertRaises(ValueError, ZstdCompressor, options=d1)
283+
with self.assertRaises(OverflowError):
284+
ZstdCompressor(options=d1)
285+
# smaller than signed int
286+
d1[CompressionParameter.ldm_bucket_size_log] = -(2**31)-1
287+
with self.assertRaises(OverflowError):
288+
ZstdCompressor(options=d1)
267289

268290
# out of bounds compression level
269291
level_min, level_max = CompressionParameter.compression_level.bounds()
@@ -399,17 +421,17 @@ def test_simple_decompress_bad_args(self):
399421
self.assertRaises(TypeError, ZstdDecompressor, options='abc')
400422
self.assertRaises(TypeError, ZstdDecompressor, options=b'abc')
401423

402-
with self.assertRaises(ValueError):
424+
with self.assertRaises(OverflowError):
403425
ZstdDecompressor(options={2**31: 100})
404-
with self.assertRaises(ValueError):
426+
with self.assertRaises(OverflowError):
405427
ZstdDecompressor(options={2**1000: 100})
406-
with self.assertRaises(ValueError):
428+
with self.assertRaises(OverflowError):
407429
ZstdDecompressor(options={-(2**31)-1: 100})
408-
with self.assertRaises(ValueError):
430+
with self.assertRaises(OverflowError):
409431
ZstdDecompressor(options={-(2**1000): 100})
410-
with self.assertRaises(ValueError):
411-
ZstdDecompressor(options={0: 2**32})
412-
with self.assertRaises(ValueError):
432+
with self.assertRaises(OverflowError):
433+
ZstdDecompressor(options={0: 2**31})
434+
with self.assertRaises(OverflowError):
413435
ZstdDecompressor(options={0: -(2**1000)})
414436

415437
with self.assertRaises(ZstdError):
@@ -428,10 +450,15 @@ def test_decompress_parameters(self):
428450
d = {DecompressionParameter.window_log_max : 15}
429451
ZstdDecompressor(options=d)
430452

431-
# larger than signed int, ValueError
432453
d1 = d.copy()
454+
# larger than signed int
433455
d1[DecompressionParameter.window_log_max] = 2**31
434-
self.assertRaises(ValueError, ZstdDecompressor, None, d1)
456+
with self.assertRaises(OverflowError):
457+
ZstdDecompressor(None, d1)
458+
# smaller than signed int
459+
d1[DecompressionParameter.window_log_max] = -(2**31)-1
460+
with self.assertRaises(OverflowError):
461+
ZstdDecompressor(None, d1)
435462

436463
# out of bounds error msg
437464
options = {DecompressionParameter.window_log_max:100}
@@ -443,16 +470,16 @@ def test_decompress_parameters(self):
443470

444471
# out of bounds deecompression parameter
445472
options[DecompressionParameter.window_log_max] = 2**31
446-
with self.assertRaises(ValueError):
473+
with self.assertRaises(OverflowError):
447474
decompress(b'', options=options)
448-
options[DecompressionParameter.window_log_max] = -(2**32)-1
449-
with self.assertRaises(ValueError):
475+
options[DecompressionParameter.window_log_max] = -(2**31)-1
476+
with self.assertRaises(OverflowError):
450477
decompress(b'', options=options)
451478
options[DecompressionParameter.window_log_max] = 2**1000
452-
with self.assertRaises(ValueError):
479+
with self.assertRaises(OverflowError):
453480
decompress(b'', options=options)
454481
options[DecompressionParameter.window_log_max] = -(2**1000)
455-
with self.assertRaises(ValueError):
482+
with self.assertRaises(OverflowError):
456483
decompress(b'', options=options)
457484

458485
def test_unknown_decompression_parameter(self):
@@ -1487,7 +1514,7 @@ def test_init_bad_check(self):
14871514
with self.assertRaises(TypeError):
14881515
ZstdFile(io.BytesIO(COMPRESSED_100_PLUS_32KB), "r", options=33)
14891516

1490-
with self.assertRaises(ValueError):
1517+
with self.assertRaises(OverflowError):
14911518
ZstdFile(io.BytesIO(COMPRESSED_100_PLUS_32KB),
14921519
options={DecompressionParameter.window_log_max:2**31})
14931520

‎Modules/_zstd/compressor.c‎

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -49,14 +49,14 @@ typedef struct {
4949
#include "clinic/compressor.c.h"
5050

5151
static int
52-
_zstd_set_c_level(ZstdCompressor *self, const int level)
52+
_zstd_set_c_level(ZstdCompressor *self, int level)
5353
{
5454
/* Set integer compression level */
5555
int min_level = ZSTD_minCLevel();
5656
int max_level = ZSTD_maxCLevel();
5757
if (level < min_level || level > max_level) {
5858
PyErr_Format(PyExc_ValueError,
59-
"%zd not in valid range %d <= compression level <= %d.",
59+
"%d not in valid range %d <= compression level <= %d.",
6060
level, min_level, max_level);
6161
return -1;
6262
}
@@ -90,7 +90,8 @@ _zstd_set_c_parameters(ZstdCompressor *self, PyObject *options)
9090

9191
if (!PyDict_Check(options)) {
9292
PyErr_Format(PyExc_TypeError,
93-
"invalid type for options, expected dict");
93+
"ZstdCompressor() argument 'options' must be dict, not %T",
94+
options);
9495
return -1;
9596
}
9697

@@ -106,22 +107,16 @@ _zstd_set_c_parameters(ZstdCompressor *self, PyObject *options)
106107
}
107108

108109
Py_INCREF(key);
110+
Py_INCREF(value);
109111
int key_v = PyLong_AsInt(key);
112+
Py_DECREF(key);
110113
if (key_v == -1 && PyErr_Occurred()) {
111-
if (PyErr_ExceptionMatches(PyExc_OverflowError)) {
112-
PyErr_SetString(PyExc_ValueError,
113-
"dictionary key must be less than 2**31");
114-
}
115114
return -1;
116115
}
117116

118-
Py_INCREF(value);
119117
int value_v = PyLong_AsInt(value);
118+
Py_DECREF(value);
120119
if (value_v == -1 && PyErr_Occurred()) {
121-
if (PyErr_ExceptionMatches(PyExc_OverflowError)) {
122-
PyErr_SetString(PyExc_ValueError,
123-
"dictionary value must be less than 2**31");
124-
}
125120
return -1;
126121
}
127122

@@ -145,6 +140,8 @@ _zstd_set_c_parameters(ZstdCompressor *self, PyObject *options)
145140

146141
/* Set parameter to compression context */
147142
size_t zstd_ret = ZSTD_CCtx_setParameter(self->cctx, key_v, value_v);
143+
144+
/* Check error */
148145
if (ZSTD_isError(zstd_ret)) {
149146
set_parameter_error(mod_state, 1, key_v, value_v);
150147
return -1;
@@ -371,7 +368,7 @@ _zstd_ZstdCompressor_new_impl(PyTypeObject *type, PyObject *level,
371368
self->last_mode = ZSTD_e_end;
372369

373370
if (level != Py_None && options != Py_None) {
374-
PyErr_SetString(PyExc_RuntimeError,
371+
PyErr_SetString(PyExc_TypeError,
375372
"Only one of level or options should be used.");
376373
goto error;
377374
}
@@ -387,8 +384,8 @@ _zstd_ZstdCompressor_new_impl(PyTypeObject *type, PyObject *level,
387384
if (level_v == -1 && PyErr_Occurred()) {
388385
if (PyErr_ExceptionMatches(PyExc_OverflowError)) {
389386
PyErr_Format(PyExc_ValueError,
390-
"%zd not in valid range %d <= compression level <= %d.",
391-
level, ZSTD_minCLevel(), ZSTD_maxCLevel());
387+
"compression level not in valid range %d <= level <= %d.",
388+
ZSTD_minCLevel(), ZSTD_maxCLevel());
392389
}
393390
goto error;
394391
}

‎Modules/_zstd/decompressor.c‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,9 @@ _zstd_set_d_parameters(ZstdDecompressor *self, PyObject *options)
9797
}
9898

9999
if (!PyDict_Check(options)) {
100-
PyErr_SetString(PyExc_TypeError,
101-
"invalid type for options, expected dict");
100+
PyErr_Format(PyExc_TypeError,
101+
"ZstdDecompressor() argument 'options' must be dict, not %T",
102+
options);
102103
return -1;
103104
}
104105

@@ -114,22 +115,16 @@ _zstd_set_d_parameters(ZstdDecompressor *self, PyObject *options)
114115
}
115116

116117
Py_INCREF(key);
118+
Py_INCREF(value);
117119
int key_v = PyLong_AsInt(key);
120+
Py_DECREF(key);
118121
if (key_v == -1 && PyErr_Occurred()) {
119-
if (PyErr_ExceptionMatches(PyExc_OverflowError)) {
120-
PyErr_SetString(PyExc_ValueError,
121-
"dictionary key must be less than 2**31");
122-
}
123122
return -1;
124123
}
125124

126-
Py_INCREF(value);
127125
int value_v = PyLong_AsInt(value);
126+
Py_DECREF(value);
128127
if (value_v == -1 && PyErr_Occurred()) {
129-
if (PyErr_ExceptionMatches(PyExc_OverflowError)) {
130-
PyErr_SetString(PyExc_ValueError,
131-
"dictionary value must be less than 2**31");
132-
}
133128
return -1;
134129
}
135130

0 commit comments

Comments
 (0)