diff --git a/ext/shmop/shmop.c b/ext/shmop/shmop.c index be4f57ad27162..e2330fa7f35a2 100644 --- a/ext/shmop/shmop.c +++ b/ext/shmop/shmop.c @@ -133,25 +133,41 @@ PHP_MINFO_FUNCTION(shmop) /* {{{ gets and attaches a shared memory segment */ PHP_FUNCTION(shmop_open) { - zend_long key, mode, size; + zend_long key, permissions, size; php_shmop *shmop; struct shmid_ds shm; char *flags; size_t flags_len; - if (zend_parse_parameters(ZEND_NUM_ARGS(), "lsll", &key, &flags, &flags_len, &mode, &size) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "lsll", &key, &flags, &flags_len, &permissions, &size) == FAILURE) { RETURN_THROWS(); } if (flags_len != 1) { - zend_argument_value_error(2, "must be a valid access mode"); + zend_argument_value_error(2, "must be a valid access mode, which is one of \"a\", \"c\", \"n\", or \"w\""); RETURN_THROWS(); } + // TODO Check that permissions are valid? + if (permissions < 0) { + zend_argument_value_error(3, "must be greater or equal than 0"); + RETURN_THROWS(); + } + + if (size < 0) { + zend_argument_value_error(4, "must be greater or equal than 0"); + RETURN_THROWS(); + } + + /* There is some implications from the php.net docs that if reopening a segment both permision and size must be 0 + * Do something to check this is hold? Or just split the modes into seperate functions + */ + + object_init_ex(return_value, shmop_ce); shmop = Z_SHMOP_P(return_value); shmop->key = key; - shmop->shmflg |= mode; + shmop->shmflg |= permissions; switch (flags[0]) { @@ -173,7 +189,7 @@ PHP_FUNCTION(shmop_open) */ break; default: - zend_argument_value_error(2, "must be a valid access mode"); + zend_argument_value_error(2, "must be a valid access mode, which is one of \"a\", \"c\", \"n\", or \"w\""); goto err; } @@ -231,11 +247,15 @@ PHP_FUNCTION(shmop_read) shmop = Z_SHMOP_P(shmid); if (start < 0 || start > shmop->size) { - zend_argument_value_error(2, "must be between 0 and the segment size"); + zend_argument_value_error(2, "must be between 0 and the segment size of %d", shmop->size); RETURN_THROWS(); } - if (count < 0 || start > (ZEND_LONG_MAX - count) || start + count > shmop->size) { + if (count <= 0) { + zend_argument_value_error(3, "must be greater than 0"); + RETURN_THROWS(); + } + if (start > (ZEND_LONG_MAX - count) || start + count > shmop->size) { zend_argument_value_error(3, "is out of range"); RETURN_THROWS(); } @@ -280,7 +300,6 @@ PHP_FUNCTION(shmop_size) PHP_FUNCTION(shmop_write) { php_shmop *shmop; - zend_long writesize; zend_long offset; zend_string *data; zval *shmid; @@ -297,14 +316,19 @@ PHP_FUNCTION(shmop_write) } if (offset < 0 || offset > shmop->size) { - zend_argument_value_error(3, "is out of range"); + zend_argument_value_error(3, "must be between 0 and the segment size of %d", shmop->size); + RETURN_THROWS(); + } + + if (ZSTR_LEN(data) > shmop->size - (size_t)offset) { + zend_argument_value_error(2, "cannot write data of size %zu from offset " ZEND_LONG_FMT + " into a segment of size %d", ZSTR_LEN(data), offset, shmop->size); RETURN_THROWS(); } - writesize = ((zend_long)ZSTR_LEN(data) < shmop->size - offset) ? (zend_long)ZSTR_LEN(data) : shmop->size - offset; - memcpy(shmop->addr + offset, ZSTR_VAL(data), writesize); + memcpy(shmop->addr + ((size_t)offset), ZSTR_VAL(data), ZSTR_LEN(data)); - RETURN_LONG(writesize); + RETURN_LONG(ZSTR_LEN(data)); } /* }}} */ @@ -321,7 +345,7 @@ PHP_FUNCTION(shmop_delete) shmop = Z_SHMOP_P(shmid); if (shmctl(shmop->shmid, IPC_RMID, NULL)) { - php_error_docref(NULL, E_WARNING, "Can't mark segment for deletion (are you the owner?)"); + php_error_docref(NULL, E_WARNING, "Cannot mark segment for deletion"); RETURN_FALSE; } diff --git a/ext/shmop/tests/002.phpt b/ext/shmop/tests/002.phpt deleted file mode 100644 index 56aea8fcdcb9f..0000000000000 --- a/ext/shmop/tests/002.phpt +++ /dev/null @@ -1,87 +0,0 @@ ---TEST-- -shmop extension error messages ---CREDITS-- -edgarsandi - ---EXTENSIONS-- -shmop ---FILE-- -getMessage() . "\n"; -} - -try { - shmop_open(1338, 'b', 0644, 1024); -} catch (ValueError $exception) { - echo $exception->getMessage() . "\n"; -} - -// Warning outputs: Unable to attach or create shared memory segment -var_dump(shmop_open(0, 'a', 0644, 1024)); - -// Shared memory segment size must be greater than zero -try { - shmop_open(0, 'a', 0644, 1024); -} catch (ValueError $exception) { - echo $exception->getMessage() . "\n"; -} - -//Shared memory segment size must be greater than zero -try { - shmop_open(1338, "c", 0666, 0); -} catch (ValueError $exception) { - echo $exception->getMessage() . "\n"; -} - -echo PHP_EOL, '## shmop_read function tests ##', PHP_EOL; -// Start is out of range -$shm_id = shmop_open(1338, 'n', 0600, 1024); -try { - shmop_read($shm_id, -10, 0); -} catch (ValueError $exception) { - echo $exception->getMessage() . "\n"; -} -shmop_delete($shm_id); - -// Count is out of range -$shm_id = shmop_open(1339, 'n', 0600, 1024); -try { - shmop_read($shm_id, 0, -10); -} catch (ValueError $exception) { - echo $exception->getMessage() . "\n"; -} -shmop_delete($shm_id); - -echo PHP_EOL, '## shmop_write function tests ##', PHP_EOL; -// Offset out of range -$shm_id = shmop_open(1340, 'n', 0600, 1024); -try { - shmop_write($shm_id, 'text to try write', -10); -} catch (ValueError $exception) { - echo $exception->getMessage() . "\n"; -} -shmop_delete($shm_id); -?> ---EXPECTF-- -## shmop_open function tests ## -shmop_open(): Argument #2 ($mode) must be a valid access mode -shmop_open(): Argument #2 ($mode) must be a valid access mode - -Warning: shmop_open(): Unable to attach or create shared memory segment "%s" in %s on line %d -bool(false) - -Warning: shmop_open(): Unable to attach or create shared memory segment "%s" in %s on line %d -shmop_open(): Argument #4 ($size) must be greater than 0 for the "c" and "n" access modes - -## shmop_read function tests ## -shmop_read(): Argument #2 ($offset) must be between 0 and the segment size -shmop_read(): Argument #3 ($size) is out of range - -## shmop_write function tests ## -shmop_write(): Argument #3 ($offset) is out of range diff --git a/ext/shmop/tests/cannot_construct_class_via_new.phpt b/ext/shmop/tests/cannot_construct_class_via_new.phpt new file mode 100644 index 0000000000000..f8bf1805a1db5 --- /dev/null +++ b/ext/shmop/tests/cannot_construct_class_via_new.phpt @@ -0,0 +1,14 @@ +--TEST-- +Shmop class must not be instantiatable +--EXTENSIONS-- +shmop +--FILE-- +getMessage(), PHP_EOL; +} +?> +--EXPECT-- +Error: Cannot directly construct Shmop, use shmop_open() instead diff --git a/ext/shmop/tests/shmop_open_errors.phpt b/ext/shmop/tests/shmop_open_errors.phpt new file mode 100644 index 0000000000000..37ede98354493 --- /dev/null +++ b/ext/shmop/tests/shmop_open_errors.phpt @@ -0,0 +1,45 @@ +--TEST-- +shmop_open errors +--EXTENSIONS-- +shmop +--FILE-- +getMessage(), PHP_EOL; +} +try { + $shmop = shmop_open($key, 'wrong_mode', 0, 0); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +try { + $shmop = shmop_open($key, 'q', 0, 0); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +try { + $shmop = shmop_open($key, 'r', -1, 0); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +try { + $shmop = shmop_open($key, 'r', 0, -1); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +try { + $shmop = shmop_open($key, 'c', 0, 0); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +?> +--EXPECT-- +ValueError: shmop_open(): Argument #2 ($mode) must be a valid access mode, which is one of "a", "c", "n", or "w" +ValueError: shmop_open(): Argument #2 ($mode) must be a valid access mode, which is one of "a", "c", "n", or "w" +ValueError: shmop_open(): Argument #2 ($mode) must be a valid access mode, which is one of "a", "c", "n", or "w" +ValueError: shmop_open(): Argument #3 ($permissions) must be greater or equal than 0 +ValueError: shmop_open(): Argument #4 ($size) must be greater or equal than 0 +ValueError: shmop_open(): Argument #4 ($size) must be greater than 0 for the "c" and "n" access modes diff --git a/ext/shmop/tests/shmop_open_n_identical_segment.phpt b/ext/shmop/tests/shmop_open_n_identical_segment.phpt new file mode 100644 index 0000000000000..be3a5dd4f15e2 --- /dev/null +++ b/ext/shmop/tests/shmop_open_n_identical_segment.phpt @@ -0,0 +1,13 @@ +--TEST-- +Cannot create existing segment with n mode +--EXTENSIONS-- +shmop +--FILE-- + +--EXPECTF-- +Warning: shmop_open(): Unable to attach or create shared memory segment "File exists" in %s on line %d diff --git a/ext/shmop/tests/shmop_open_warnings.phpt b/ext/shmop/tests/shmop_open_warnings.phpt new file mode 100644 index 0000000000000..97430afc3f4ef --- /dev/null +++ b/ext/shmop/tests/shmop_open_warnings.phpt @@ -0,0 +1,26 @@ +--TEST-- +shmop extension error messages +--CREDITS-- +edgarsandi - +--EXTENSIONS-- +shmop +--FILE-- +getMessage() . "\n"; +} +?> +--EXPECTF-- + +Warning: shmop_open(): Unable to attach or create shared memory segment "%s" in %s on line %d +bool(false) + +Warning: shmop_open(): Unable to attach or create shared memory segment "%s" in %s on line %d +bool(false) diff --git a/ext/shmop/tests/shmop_read_errors.phpt b/ext/shmop/tests/shmop_read_errors.phpt new file mode 100644 index 0000000000000..f627f65d70f59 --- /dev/null +++ b/ext/shmop/tests/shmop_read_errors.phpt @@ -0,0 +1,41 @@ +--TEST-- +shmop_read errors +--EXTENSIONS-- +shmop +--FILE-- +getMessage(), PHP_EOL; +} +try { + $shmop = shmop_read($shm_id, 1400, 100); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +try { + $shmop = shmop_read($shm_id, 0, -1); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +try { + $shmop = shmop_read($shm_id, 0, 1400); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +try { + $shmop = shmop_read($shm_id, 1000, 25); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +shmop_delete($shm_id); +?> +--EXPECT-- +ValueError: shmop_read(): Argument #2 ($offset) must be between 0 and the segment size of 1024 +ValueError: shmop_read(): Argument #2 ($offset) must be between 0 and the segment size of 1024 +ValueError: shmop_read(): Argument #3 ($size) must be greater than 0 +ValueError: shmop_read(): Argument #3 ($size) is out of range +ValueError: shmop_read(): Argument #3 ($size) is out of range diff --git a/ext/shmop/tests/shmop_write_errors.phpt b/ext/shmop/tests/shmop_write_errors.phpt new file mode 100644 index 0000000000000..fa77bf355fb38 --- /dev/null +++ b/ext/shmop/tests/shmop_write_errors.phpt @@ -0,0 +1,25 @@ +--TEST-- +shmop_write errors +--EXTENSIONS-- +shmop +--FILE-- +getMessage(), PHP_EOL; +} +try { + var_dump(shmop_write($shm_id, 'text to try write', 20)); + var_dump(shmop_read($shm_id, 0, 16)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +shmop_delete($shm_id); +?> +--EXPECT-- +ValueError: shmop_write(): Argument #3 ($offset) must be between 0 and the segment size of 16 +ValueError: shmop_write(): Argument #3 ($offset) must be between 0 and the segment size of 16 diff --git a/ext/shmop/tests/shmop_write_partial.phpt b/ext/shmop/tests/shmop_write_partial.phpt new file mode 100644 index 0000000000000..df24395defdbb --- /dev/null +++ b/ext/shmop/tests/shmop_write_partial.phpt @@ -0,0 +1,26 @@ +--TEST-- +shmop_write with a partial write +--EXTENSIONS-- +shmop +--FILE-- +getMessage(), PHP_EOL; +} +shmop_delete($shm_id); +?> +--EXPECT-- +int(16) +string(16) "aaaaaaaaaaaaaaaa" +ValueError: shmop_write(): Argument #2 ($data) cannot write data of size 8 from offset 11 into a segment of size 16