Skip to content

gh-132983: Split _zstd_set_c_parameters #133921

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 17 commits into from
May 28, 2025
Merged
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 52 additions & 65 deletions Lib/test/test_zstd.py
Original file line number Diff line number Diff line change
@@ -64,6 +64,10 @@

SUPPORT_MULTITHREADING = False

C_INT_MIN = -(2**31)
C_INT_MAX = (2**31) - 1


def setUpModule():
global SUPPORT_MULTITHREADING
SUPPORT_MULTITHREADING = CompressionParameter.nb_workers.bounds() != (0, 0)
@@ -195,38 +199,21 @@ def test_simple_compress_bad_args(self):
self.assertRaises(TypeError, ZstdCompressor, zstd_dict=b"abcd1234")
self.assertRaises(TypeError, ZstdCompressor, zstd_dict={1: 2, 3: 4})

# valid compression level range is [-(1<<17), 22]
with self.assertRaises(ValueError) as cm:
ZstdCompressor(23)
self.assertEqual(
str(cm.exception),
'23 not in valid range -131072 <= compression level <= 22.',
)
with self.assertRaises(ValueError) as cm:
ZstdCompressor(-(1<<17)-1)
self.assertEqual(-(1<<17)-1, -131073)
self.assertEqual(
str(cm.exception),
'-131073 not in valid range -131072 <= compression level <= 22.',
)
with self.assertRaises(ValueError) as cm:
ZstdCompressor(2**31)
self.assertEqual(
str(cm.exception),
'compression level not in valid range -131072 <= level <= 22.',
)
with self.assertRaises(ValueError) as cm:
# valid range for compression level is [-(1<<17), 22]
msg = 'compression level {} not in valid range -131072 <= level <= 22'
with self.assertRaisesRegex(ValueError, msg.format(C_INT_MAX)):
ZstdCompressor(C_INT_MAX)
with self.assertRaisesRegex(ValueError, msg.format(C_INT_MIN)):
ZstdCompressor(C_INT_MIN)
msg = 'compression level not in valid range -131072 <= level <= 22'
with self.assertRaisesRegex(ValueError, msg):
ZstdCompressor(level=-(2**1000))
self.assertEqual(
str(cm.exception),
'compression level not in valid range -131072 <= level <= 22.',
)
with self.assertRaises(ValueError):
with self.assertRaisesRegex(ValueError, msg):
ZstdCompressor(level=(2**1000))

with self.assertRaises(ZstdError):
with self.assertRaises(ValueError):
ZstdCompressor(options={CompressionParameter.window_log: 100})
with self.assertRaises(ZstdError):
with self.assertRaises(ValueError):
ZstdCompressor(options={3333: 100})

# Method bad arguments
@@ -279,12 +266,12 @@ def test_compress_parameters(self):

d1 = d.copy()
# larger than signed int
d1[CompressionParameter.ldm_bucket_size_log] = 2**31
with self.assertRaises(OverflowError):
d1[CompressionParameter.ldm_bucket_size_log] = C_INT_MAX
with self.assertRaises(ValueError):
ZstdCompressor(options=d1)
# smaller than signed int
d1[CompressionParameter.ldm_bucket_size_log] = -(2**31)-1
with self.assertRaises(OverflowError):
d1[CompressionParameter.ldm_bucket_size_log] = C_INT_MIN
with self.assertRaises(ValueError):
ZstdCompressor(options=d1)

# out of bounds compression level
@@ -315,19 +302,19 @@ def test_compress_parameters(self):

# out of bounds error msg
option = {CompressionParameter.window_log:100}
with self.assertRaisesRegex(ZstdError,
(r'Error when setting zstd compression parameter "window_log", '
r'it should \d+ <= value <= \d+, provided value is 100\. '
r'\((?:32|64)-bit build\)')):
with self.assertRaisesRegex(
ValueError,
"compression parameter 'window_log' received an illegal value 100; "
r'the valid range is \d+ <= value <= \d+',
):
compress(b'', options=option)

def test_unknown_compression_parameter(self):
KEY = 100001234
option = {CompressionParameter.compression_level: 10,
KEY: 200000000}
pattern = (r'Invalid zstd compression parameter.*?'
fr'"unknown parameter \(key {KEY}\)"')
with self.assertRaisesRegex(ZstdError, pattern):
pattern = rf"invalid compression parameter 'unknown parameter \(key {KEY}\)'"
with self.assertRaisesRegex(ValueError, pattern):
ZstdCompressor(options=option)

@unittest.skipIf(not SUPPORT_MULTITHREADING,
@@ -421,22 +408,22 @@ def test_simple_decompress_bad_args(self):
self.assertRaises(TypeError, ZstdDecompressor, options='abc')
self.assertRaises(TypeError, ZstdDecompressor, options=b'abc')

with self.assertRaises(OverflowError):
ZstdDecompressor(options={2**31: 100})
with self.assertRaises(ValueError):
ZstdDecompressor(options={C_INT_MAX: 100})
with self.assertRaises(ValueError):
ZstdDecompressor(options={C_INT_MIN: 100})
with self.assertRaises(ValueError):
ZstdDecompressor(options={0: C_INT_MAX})
with self.assertRaises(OverflowError):
ZstdDecompressor(options={2**1000: 100})
with self.assertRaises(OverflowError):
ZstdDecompressor(options={-(2**31)-1: 100})
with self.assertRaises(OverflowError):
ZstdDecompressor(options={-(2**1000): 100})
with self.assertRaises(OverflowError):
ZstdDecompressor(options={0: 2**31})
with self.assertRaises(OverflowError):
ZstdDecompressor(options={0: -(2**1000)})

with self.assertRaises(ZstdError):
with self.assertRaises(ValueError):
ZstdDecompressor(options={DecompressionParameter.window_log_max: 100})
with self.assertRaises(ZstdError):
with self.assertRaises(ValueError):
ZstdDecompressor(options={3333: 100})

empty = compress(b'')
@@ -452,28 +439,29 @@ def test_decompress_parameters(self):

d1 = d.copy()
# larger than signed int
d1[DecompressionParameter.window_log_max] = 2**31
with self.assertRaises(OverflowError):
d1[DecompressionParameter.window_log_max] = C_INT_MAX
with self.assertRaises(ValueError):
ZstdDecompressor(None, d1)
# smaller than signed int
d1[DecompressionParameter.window_log_max] = -(2**31)-1
with self.assertRaises(OverflowError):
d1[DecompressionParameter.window_log_max] = C_INT_MIN
with self.assertRaises(ValueError):
ZstdDecompressor(None, d1)

# out of bounds error msg
options = {DecompressionParameter.window_log_max:100}
with self.assertRaisesRegex(ZstdError,
(r'Error when setting zstd decompression parameter "window_log_max", '
r'it should \d+ <= value <= \d+, provided value is 100\. '
r'\((?:32|64)-bit build\)')):
with self.assertRaisesRegex(
ValueError,
"decompression parameter 'window_log_max' received an illegal value 100; "
r'the valid range is \d+ <= value <= \d+',
):
decompress(b'', options=options)

# out of bounds deecompression parameter
options[DecompressionParameter.window_log_max] = 2**31
with self.assertRaises(OverflowError):
options[DecompressionParameter.window_log_max] = C_INT_MAX
with self.assertRaises(ValueError):
decompress(b'', options=options)
options[DecompressionParameter.window_log_max] = -(2**31)-1
with self.assertRaises(OverflowError):
options[DecompressionParameter.window_log_max] = C_INT_MIN
with self.assertRaises(ValueError):
decompress(b'', options=options)
options[DecompressionParameter.window_log_max] = 2**1000
with self.assertRaises(OverflowError):
@@ -486,9 +474,8 @@ def test_unknown_decompression_parameter(self):
KEY = 100001234
options = {DecompressionParameter.window_log_max: DecompressionParameter.window_log_max.bounds()[1],
KEY: 200000000}
pattern = (r'Invalid zstd decompression parameter.*?'
fr'"unknown parameter \(key {KEY}\)"')
with self.assertRaisesRegex(ZstdError, pattern):
pattern = rf"invalid decompression parameter 'unknown parameter \(key {KEY}\)'"
with self.assertRaisesRegex(ValueError, pattern):
ZstdDecompressor(options=options)

def test_decompress_epilogue_flags(self):
@@ -1506,9 +1493,9 @@ def test_init_bad_check(self):
with self.assertRaises(TypeError):
ZstdFile(io.BytesIO(), "w", level='asd')
# CHECK_UNKNOWN and anything above CHECK_ID_MAX should be invalid.
with self.assertRaises(ZstdError):
with self.assertRaises(ValueError):
ZstdFile(io.BytesIO(), "w", options={999:9999})
with self.assertRaises(ZstdError):
with self.assertRaises(ValueError):
ZstdFile(io.BytesIO(), "w", options={CompressionParameter.window_log:99})

with self.assertRaises(TypeError):
@@ -1518,7 +1505,7 @@ def test_init_bad_check(self):
ZstdFile(io.BytesIO(COMPRESSED_100_PLUS_32KB),
options={DecompressionParameter.window_log_max:2**31})

with self.assertRaises(ZstdError):
with self.assertRaises(ValueError):
ZstdFile(io.BytesIO(COMPRESSED_100_PLUS_32KB),
options={444:333})

25 changes: 9 additions & 16 deletions Modules/_zstd/_zstdmodule.c
Original file line number Diff line number Diff line change
@@ -103,16 +103,13 @@ static const ParameterInfo dp_list[] = {
};

void
set_parameter_error(const _zstd_state* const state, int is_compress,
int key_v, int value_v)
set_parameter_error(int is_compress, int key_v, int value_v)
{
ParameterInfo const *list;
int list_size;
char const *name;
char *type;
ZSTD_bounds bounds;
int i;
char pos_msg[128];
char pos_msg[64];

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

/* Find parameter's name */
name = NULL;
for (i = 0; i < list_size; i++) {
char const *name = NULL;
for (int i = 0; i < list_size; i++) {
if (key_v == (list+i)->parameter) {
name = (list+i)->parameter_name;
break;
@@ -149,20 +146,16 @@ set_parameter_error(const _zstd_state* const state, int is_compress,
bounds = ZSTD_dParam_getBounds(key_v);
}
if (ZSTD_isError(bounds.error)) {
PyErr_Format(state->ZstdError,
"Invalid zstd %s parameter \"%s\".",
PyErr_Format(PyExc_ValueError, "invalid %s parameter '%s'",
type, name);
return;
}

/* Error message */
PyErr_Format(state->ZstdError,
"Error when setting zstd %s parameter \"%s\", it "
"should %d <= value <= %d, provided value is %d. "
"(%d-bit build)",
type, name,
bounds.lowerBound, bounds.upperBound, value_v,
8*(int)sizeof(Py_ssize_t));
PyErr_Format(PyExc_ValueError,
"%s parameter '%s' received an illegal value %d; "
"the valid range is %d <= value <= %d",
type, name, value_v, bounds.lowerBound, bounds.upperBound);
}

static inline _zstd_state*
3 changes: 1 addition & 2 deletions Modules/_zstd/_zstdmodule.h
Original file line number Diff line number Diff line change
@@ -49,7 +49,6 @@ set_zstd_error(const _zstd_state* const state,
const error_type type, size_t zstd_ret);

extern void
set_parameter_error(const _zstd_state* const state, int is_compress,
int key_v, int value_v);
set_parameter_error(int is_compress, int key_v, int value_v);

#endif // !ZSTD_MODULE_H
7 changes: 4 additions & 3 deletions Modules/_zstd/compressor.c
Original file line number Diff line number Diff line change
@@ -56,7 +56,7 @@ _zstd_set_c_level(ZstdCompressor *self, int level)
int max_level = ZSTD_maxCLevel();
if (level < min_level || level > max_level) {
PyErr_Format(PyExc_ValueError,
"%d not in valid range %d <= compression level <= %d.",
"compression level %d not in valid range %d <= level <= %d",
level, min_level, max_level);
return -1;
}
@@ -111,6 +111,7 @@ _zstd_set_c_parameters(ZstdCompressor *self, PyObject *options)
int key_v = PyLong_AsInt(key);
Py_DECREF(key);
if (key_v == -1 && PyErr_Occurred()) {
Py_DECREF(value);
return -1;
}

@@ -143,7 +144,7 @@ _zstd_set_c_parameters(ZstdCompressor *self, PyObject *options)

/* Check error */
if (ZSTD_isError(zstd_ret)) {
set_parameter_error(mod_state, 1, key_v, value_v);
set_parameter_error(1, key_v, value_v);
return -1;
}
}
@@ -377,7 +378,7 @@ _zstd_ZstdCompressor_new_impl(PyTypeObject *type, PyObject *level,
if (level_v == -1 && PyErr_Occurred()) {
if (PyErr_ExceptionMatches(PyExc_OverflowError)) {
PyErr_Format(PyExc_ValueError,
"compression level not in valid range %d <= level <= %d.",
"compression level not in valid range %d <= level <= %d",
ZSTD_minCLevel(), ZSTD_maxCLevel());
}
goto error;
2 changes: 1 addition & 1 deletion Modules/_zstd/decompressor.c
Original file line number Diff line number Diff line change
@@ -131,7 +131,7 @@ _zstd_set_d_parameters(ZstdDecompressor *self, PyObject *options)

/* Check error */
if (ZSTD_isError(zstd_ret)) {
set_parameter_error(mod_state, 0, key_v, value_v);
set_parameter_error(0, key_v, value_v);
return -1;
}
}
Loading
Oops, something went wrong.