Skip to content

Commit

Permalink
Promote warnings to exceptions in ext/phar
Browse files Browse the repository at this point in the history 
Closes GH-6008
  • Loading branch information
@kocsismate
kocsismate committed Aug 25, 2020
1 parent f068fbc commit be5ba20
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 30 deletions.
15 changes: 10 additions & 5 deletions ext/phar/func_interceptors.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ PHAR_FUNC(phar_file_get_contents) /* {{{ */
zend_bool use_include_path = 0;
php_stream *stream;
zend_long offset = -1;
zend_long maxlen = PHP_STREAM_COPY_ALL;
zend_long maxlen;
zend_bool maxlen_is_null = 1;
zval *zcontext = NULL;

if (!PHAR_G(intercepted)) {
Expand All @@ -110,10 +111,14 @@ PHAR_FUNC(phar_file_get_contents) /* {{{ */
}

/* Parse arguments */
if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "p|br!ll", &filename, &filename_len, &use_include_path, &zcontext, &offset, &maxlen) == FAILURE) {
if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, ZEND_NUM_ARGS(), "p|br!ll!", &filename, &filename_len, &use_include_path, &zcontext, &offset, &maxlen, &maxlen_is_null) == FAILURE) {
goto skip_phar;
}

if (maxlen_is_null) {
maxlen = (ssize_t) PHP_STREAM_COPY_ALL;
}

if (use_include_path || (!IS_ABSOLUTE_PATH(filename, filename_len) && !strstr(filename, "://"))) {
char *arch, *entry, *fname;
zend_string *entry_str = NULL;
Expand All @@ -135,10 +140,10 @@ PHAR_FUNC(phar_file_get_contents) /* {{{ */
/* fopen within phar, if :// is not in the url, then prepend phar://<archive>/ */
entry_len = filename_len;

if (ZEND_NUM_ARGS() == 5 && maxlen < 0) {
if (!maxlen_is_null && maxlen < 0) {
efree(arch);
php_error_docref(NULL, E_WARNING, "Length must be greater than or equal to zero");
RETURN_FALSE;
zend_argument_value_error(5, "must be greater than or equal to 0");
RETURN_THROWS();
}

/* retrieving a file defaults to within the current directory, so use this if possible */
Expand Down
8 changes: 4 additions & 4 deletions ext/phar/phar_object.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2918,7 +2918,7 @@ PHP_METHOD(Phar, setDefaultStub)
size_t index_len = 0, webindex_len = 0;
int created_stub = 0;

if (zend_parse_parameters(ZEND_NUM_ARGS(), "|s!s", &index, &index_len, &webindex, &webindex_len) == FAILURE) {
if (zend_parse_parameters(ZEND_NUM_ARGS(), "|s!s!", &index, &index_len, &webindex, &webindex_len) == FAILURE) {
RETURN_THROWS();
}

Expand All @@ -2935,9 +2935,9 @@ PHP_METHOD(Phar, setDefaultStub)
RETURN_THROWS();
}

if (ZEND_NUM_ARGS() > 0 && (phar_obj->archive->is_tar || phar_obj->archive->is_zip)) {
php_error_docref(NULL, E_WARNING, "Method accepts no arguments for a tar- or zip-based phar stub, %d given", ZEND_NUM_ARGS());
RETURN_FALSE;
if ((index || webindex) && (phar_obj->archive->is_tar || phar_obj->archive->is_zip)) {
zend_argument_value_error(index ? 1 : 2, "must be null for a tar- or zip-based phar stub, string given");
RETURN_THROWS();
}

if (PHAR_G(readonly)) {
Expand Down
4 changes: 2 additions & 2 deletions ext/phar/phar_object.stub.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ public function offsetUnset($entry) {}
public function setAlias(string $alias) {}

/** @return bool */
public function setDefaultStub(?string $index = null, string $webindex = UNKNOWN) {}
public function setDefaultStub(?string $index = null, ?string $webindex = null) {}

/**
* @param mixed $metadata
Expand Down Expand Up @@ -398,7 +398,7 @@ public function setAlias(string $alias) {}
* @return bool
* @alias Phar::setDefaultStub
*/
public function setDefaultStub(?string $index = null, string $webindex = UNKNOWN) {}
public function setDefaultStub(?string $index = null, ?string $webindex = null) {}

/**
* @param mixed $metadata
Expand Down
4 changes: 2 additions & 2 deletions ext/phar/phar_object_arginfo.h
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* This is a generated file, edit the .stub.php file instead.
* Stub hash: f25efd47b496a7d06a30c77911a565a49e383bce */
* Stub hash: be0f8bcd0ef8fdac59700160dff7d0beb210aa48 */

ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Phar___construct, 0, 0, 1)
ZEND_ARG_TYPE_INFO(0, filename, IS_STRING, 0)
Expand Down Expand Up @@ -125,7 +125,7 @@ ZEND_END_ARG_INFO()

ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Phar_setDefaultStub, 0, 0, 0)
ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, index, IS_STRING, 1, "null")
ZEND_ARG_TYPE_INFO(0, webindex, IS_STRING, 0)
ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, webindex, IS_STRING, 1, "null")
ZEND_END_ARG_INFO()

ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Phar_setMetadata, 0, 0, 1)
Expand Down
27 changes: 21 additions & 6 deletions ext/phar/tests/fgc_edgecases.phpt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@ mkdir($pname . '/oops');
file_put_contents($pname . '/foo/hi', '<?php
echo file_get_contents("foo/" . basename(__FILE__));
$context = stream_context_create();
file_get_contents("./hi", 0, $context, 0, -1);
try {
file_get_contents("./hi", 0, $context, 0, -1);
} catch (ValueError $exception) {
echo $exception->getMessage() . "\n";
}
echo file_get_contents("fgc_edgecases.txt");
set_include_path("' . addslashes(__DIR__) . '");
echo file_get_contents("fgc_edgecases.txt", true);
Expand All @@ -53,7 +57,11 @@ blah
<?php
echo file_get_contents("foo/" . basename(__FILE__));
$context = stream_context_create();
file_get_contents("./hi", 0, $context, 0, -1);
try {
file_get_contents("./hi", 0, $context, 0, -1);
} catch (ValueError $exception) {
echo $exception->getMessage() . "\n";
}
echo file_get_contents("fgc_edgecases.txt");
set_include_path("%stests");
echo file_get_contents("fgc_edgecases.txt", true);
Expand All @@ -63,14 +71,17 @@ echo file_get_contents("./hi", 0, $context, 50000);
echo file_get_contents("./hi");
echo file_get_contents("./hi", 0, $context, 0, 0);
?>

Warning: file_get_contents(): Length must be greater than or equal to zero in phar://%sfgc_edgecases.phar.php/foo/hi on line %d
file_get_contents(): Argument #5 ($maxlen) must be greater than or equal to 0
test
test
<?php
echo file_get_contents("foo/" . basename(__FILE__));
$context = stream_context_create();
file_get_contents("./hi", 0, $context, 0, -1);
try {
file_get_contents("./hi", 0, $context, 0, -1);
} catch (ValueError $exception) {
echo $exception->getMessage() . "\n";
}
echo file_get_contents("fgc_edgecases.txt");
set_include_path("%stests");
echo file_get_contents("fgc_edgecases.txt", true);
Expand All @@ -87,7 +98,11 @@ Warning: file_get_contents(): Failed to seek to position 50000 in the stream in
<?php
echo file_get_contents("foo/" . basename(__FILE__));
$context = stream_context_create();
file_get_contents("./hi", 0, $context, 0, -1);
try {
file_get_contents("./hi", 0, $context, 0, -1);
} catch (ValueError $exception) {
echo $exception->getMessage() . "\n";
}
echo file_get_contents("fgc_edgecases.txt");
set_include_path("%stests");
echo file_get_contents("fgc_edgecases.txt", true);
Expand Down
20 changes: 14 additions & 6 deletions ext/phar/tests/tar/phar_setdefaultstub.phpt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,28 @@ echo "==========================================================================

try {
$phar->setDefaultStub('my/custom/thingy.php');
} catch(ValueError $e) {
echo $e->getMessage(). "\n";
}

try {
$phar->stopBuffering();
} catch(Exception $e) {
echo $e->getMessage(). "\n";
}

var_dump($phar->getStub());

echo "============================================================================\n";
echo "============================================================================\n";


try {
$phar->setDefaultStub('my/custom/thingy.php', 'the/web.php');
} catch(ValueError $e) {
echo $e->getMessage(). "\n";
}

try {
$phar->stopBuffering();
} catch(Exception $e) {
echo $e->getMessage(). "\n";
Expand All @@ -57,7 +67,7 @@ var_dump($phar->getStub());
<?php
unlink(__DIR__ . '/' . basename(__FILE__, '.clean.php') . '.phar.tar');
?>
--EXPECTF--
--EXPECT--
string(51) "<?php echo "Hello World\n"; __HALT_COMPILER(); ?>
"
============================================================================
Expand All @@ -66,13 +76,11 @@ string(60) "<?php // tar-based phar archive stub file
__HALT_COMPILER();"
============================================================================
============================================================================
Warning: Phar::setDefaultStub(): Method accepts no arguments for a tar- or zip-based phar stub, 1 given in %sphar_setdefaultstub.php on line %d
Phar::setDefaultStub(): Argument #1 ($index) must be null for a tar- or zip-based phar stub, string given
string(60) "<?php // tar-based phar archive stub file
__HALT_COMPILER();"
============================================================================
============================================================================
Warning: Phar::setDefaultStub(): Method accepts no arguments for a tar- or zip-based phar stub, 2 given in %sphar_setdefaultstub.php on line %d
Phar::setDefaultStub(): Argument #1 ($index) must be null for a tar- or zip-based phar stub, string given
string(60) "<?php // tar-based phar archive stub file
__HALT_COMPILER();"
18 changes: 13 additions & 5 deletions ext/phar/tests/zip/phar_setdefaultstub.phpt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ echo "==========================================================================

try {
$phar->setDefaultStub('my/custom/thingy.php');
} catch(Error $e) {
echo $e->getMessage(). "\n";
}

try {
$phar->stopBuffering();
} catch(Exception $e) {
echo $e->getMessage(). "\n";
Expand All @@ -45,6 +50,11 @@ echo "==========================================================================

try {
$phar->setDefaultStub('my/custom/thingy.php', 'the/web.php');
} catch(ValueError $e) {
echo $e->getMessage(). "\n";
}

try {
$phar->stopBuffering();
} catch(Exception $e) {
echo $e->getMessage(). "\n";
Expand All @@ -57,7 +67,7 @@ var_dump($phar->getStub());
<?php
unlink(__DIR__ . '/' . basename(__FILE__, '.clean.php') . '.phar.zip');
?>
--EXPECTF--
--EXPECT--
string(51) "<?php echo "Hello World\n"; __HALT_COMPILER(); ?>
"
============================================================================
Expand All @@ -66,13 +76,11 @@ string(60) "<?php // zip-based phar archive stub file
__HALT_COMPILER();"
============================================================================
============================================================================
Warning: Phar::setDefaultStub(): Method accepts no arguments for a tar- or zip-based phar stub, 1 given in %sphar_setdefaultstub.php on line %d
Phar::setDefaultStub(): Argument #1 ($index) must be null for a tar- or zip-based phar stub, string given
string(60) "<?php // zip-based phar archive stub file
__HALT_COMPILER();"
============================================================================
============================================================================
Warning: Phar::setDefaultStub(): Method accepts no arguments for a tar- or zip-based phar stub, 2 given in %sphar_setdefaultstub.php on line %d
Phar::setDefaultStub(): Argument #1 ($index) must be null for a tar- or zip-based phar stub, string given
string(60) "<?php // zip-based phar archive stub file
__HALT_COMPILER();"

0 comments on commit be5ba20

Please sign in to comment.