Skip to content

Commit be5ba20

Browse files
committed
Promote warnings to exceptions in ext/phar
Closes GH-6008
1 parent f068fbc commit be5ba20

7 files changed

+66
-30
lines changed

ext/phar/func_interceptors.c

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,8 @@ PHAR_FUNC(phar_file_get_contents) /* {{{ */
9797
zend_bool use_include_path = 0;
9898
php_stream *stream;
9999
zend_long offset = -1;
100-
zend_long maxlen = PHP_STREAM_COPY_ALL;
100+
zend_long maxlen;
101+
zend_bool maxlen_is_null = 1;
101102
zval *zcontext = NULL;
102103

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

112113
/* Parse arguments */
113-
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) {
114+
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) {
114115
goto skip_phar;
115116
}
116117

118+
if (maxlen_is_null) {
119+
maxlen = (ssize_t) PHP_STREAM_COPY_ALL;
120+
}
121+
117122
if (use_include_path || (!IS_ABSOLUTE_PATH(filename, filename_len) && !strstr(filename, "://"))) {
118123
char *arch, *entry, *fname;
119124
zend_string *entry_str = NULL;
@@ -135,10 +140,10 @@ PHAR_FUNC(phar_file_get_contents) /* {{{ */
135140
/* fopen within phar, if :// is not in the url, then prepend phar://<archive>/ */
136141
entry_len = filename_len;
137142

138-
if (ZEND_NUM_ARGS() == 5 && maxlen < 0) {
143+
if (!maxlen_is_null && maxlen < 0) {
139144
efree(arch);
140-
php_error_docref(NULL, E_WARNING, "Length must be greater than or equal to zero");
141-
RETURN_FALSE;
145+
zend_argument_value_error(5, "must be greater than or equal to 0");
146+
RETURN_THROWS();
142147
}
143148

144149
/* retrieving a file defaults to within the current directory, so use this if possible */

ext/phar/phar_object.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2918,7 +2918,7 @@ PHP_METHOD(Phar, setDefaultStub)
29182918
size_t index_len = 0, webindex_len = 0;
29192919
int created_stub = 0;
29202920

2921-
if (zend_parse_parameters(ZEND_NUM_ARGS(), "|s!s", &index, &index_len, &webindex, &webindex_len) == FAILURE) {
2921+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "|s!s!", &index, &index_len, &webindex, &webindex_len) == FAILURE) {
29222922
RETURN_THROWS();
29232923
}
29242924

@@ -2935,9 +2935,9 @@ PHP_METHOD(Phar, setDefaultStub)
29352935
RETURN_THROWS();
29362936
}
29372937

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

29432943
if (PHAR_G(readonly)) {

ext/phar/phar_object.stub.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ public function offsetUnset($entry) {}
125125
public function setAlias(string $alias) {}
126126

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

130130
/**
131131
* @param mixed $metadata
@@ -398,7 +398,7 @@ public function setAlias(string $alias) {}
398398
* @return bool
399399
* @alias Phar::setDefaultStub
400400
*/
401-
public function setDefaultStub(?string $index = null, string $webindex = UNKNOWN) {}
401+
public function setDefaultStub(?string $index = null, ?string $webindex = null) {}
402402

403403
/**
404404
* @param mixed $metadata

ext/phar/phar_object_arginfo.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* This is a generated file, edit the .stub.php file instead.
2-
* Stub hash: f25efd47b496a7d06a30c77911a565a49e383bce */
2+
* Stub hash: be0f8bcd0ef8fdac59700160dff7d0beb210aa48 */
33

44
ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Phar___construct, 0, 0, 1)
55
ZEND_ARG_TYPE_INFO(0, filename, IS_STRING, 0)
@@ -125,7 +125,7 @@ ZEND_END_ARG_INFO()
125125

126126
ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Phar_setDefaultStub, 0, 0, 0)
127127
ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, index, IS_STRING, 1, "null")
128-
ZEND_ARG_TYPE_INFO(0, webindex, IS_STRING, 0)
128+
ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, webindex, IS_STRING, 1, "null")
129129
ZEND_END_ARG_INFO()
130130

131131
ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Phar_setMetadata, 0, 0, 1)

ext/phar/tests/fgc_edgecases.phpt

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,11 @@ mkdir($pname . '/oops');
2929
file_put_contents($pname . '/foo/hi', '<?php
3030
echo file_get_contents("foo/" . basename(__FILE__));
3131
$context = stream_context_create();
32-
file_get_contents("./hi", 0, $context, 0, -1);
32+
try {
33+
file_get_contents("./hi", 0, $context, 0, -1);
34+
} catch (ValueError $exception) {
35+
echo $exception->getMessage() . "\n";
36+
}
3337
echo file_get_contents("fgc_edgecases.txt");
3438
set_include_path("' . addslashes(__DIR__) . '");
3539
echo file_get_contents("fgc_edgecases.txt", true);
@@ -53,7 +57,11 @@ blah
5357
<?php
5458
echo file_get_contents("foo/" . basename(__FILE__));
5559
$context = stream_context_create();
56-
file_get_contents("./hi", 0, $context, 0, -1);
60+
try {
61+
file_get_contents("./hi", 0, $context, 0, -1);
62+
} catch (ValueError $exception) {
63+
echo $exception->getMessage() . "\n";
64+
}
5765
echo file_get_contents("fgc_edgecases.txt");
5866
set_include_path("%stests");
5967
echo file_get_contents("fgc_edgecases.txt", true);
@@ -63,14 +71,17 @@ echo file_get_contents("./hi", 0, $context, 50000);
6371
echo file_get_contents("./hi");
6472
echo file_get_contents("./hi", 0, $context, 0, 0);
6573
?>
66-
67-
Warning: file_get_contents(): Length must be greater than or equal to zero in phar://%sfgc_edgecases.phar.php/foo/hi on line %d
74+
file_get_contents(): Argument #5 ($maxlen) must be greater than or equal to 0
6875
test
6976
test
7077
<?php
7178
echo file_get_contents("foo/" . basename(__FILE__));
7279
$context = stream_context_create();
73-
file_get_contents("./hi", 0, $context, 0, -1);
80+
try {
81+
file_get_contents("./hi", 0, $context, 0, -1);
82+
} catch (ValueError $exception) {
83+
echo $exception->getMessage() . "\n";
84+
}
7485
echo file_get_contents("fgc_edgecases.txt");
7586
set_include_path("%stests");
7687
echo file_get_contents("fgc_edgecases.txt", true);
@@ -87,7 +98,11 @@ Warning: file_get_contents(): Failed to seek to position 50000 in the stream in
8798
<?php
8899
echo file_get_contents("foo/" . basename(__FILE__));
89100
$context = stream_context_create();
90-
file_get_contents("./hi", 0, $context, 0, -1);
101+
try {
102+
file_get_contents("./hi", 0, $context, 0, -1);
103+
} catch (ValueError $exception) {
104+
echo $exception->getMessage() . "\n";
105+
}
91106
echo file_get_contents("fgc_edgecases.txt");
92107
set_include_path("%stests");
93108
echo file_get_contents("fgc_edgecases.txt", true);

ext/phar/tests/tar/phar_setdefaultstub.phpt

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,18 +33,28 @@ echo "==========================================================================
3333

3434
try {
3535
$phar->setDefaultStub('my/custom/thingy.php');
36+
} catch(ValueError $e) {
37+
echo $e->getMessage(). "\n";
38+
}
39+
40+
try {
3641
$phar->stopBuffering();
3742
} catch(Exception $e) {
3843
echo $e->getMessage(). "\n";
3944
}
40-
4145
var_dump($phar->getStub());
4246

4347
echo "============================================================================\n";
4448
echo "============================================================================\n";
4549

50+
4651
try {
4752
$phar->setDefaultStub('my/custom/thingy.php', 'the/web.php');
53+
} catch(ValueError $e) {
54+
echo $e->getMessage(). "\n";
55+
}
56+
57+
try {
4858
$phar->stopBuffering();
4959
} catch(Exception $e) {
5060
echo $e->getMessage(). "\n";
@@ -57,7 +67,7 @@ var_dump($phar->getStub());
5767
<?php
5868
unlink(__DIR__ . '/' . basename(__FILE__, '.clean.php') . '.phar.tar');
5969
?>
60-
--EXPECTF--
70+
--EXPECT--
6171
string(51) "<?php echo "Hello World\n"; __HALT_COMPILER(); ?>
6272
"
6373
============================================================================
@@ -66,13 +76,11 @@ string(60) "<?php // tar-based phar archive stub file
6676
__HALT_COMPILER();"
6777
============================================================================
6878
============================================================================
69-
70-
Warning: Phar::setDefaultStub(): Method accepts no arguments for a tar- or zip-based phar stub, 1 given in %sphar_setdefaultstub.php on line %d
79+
Phar::setDefaultStub(): Argument #1 ($index) must be null for a tar- or zip-based phar stub, string given
7180
string(60) "<?php // tar-based phar archive stub file
7281
__HALT_COMPILER();"
7382
============================================================================
7483
============================================================================
75-
76-
Warning: Phar::setDefaultStub(): Method accepts no arguments for a tar- or zip-based phar stub, 2 given in %sphar_setdefaultstub.php on line %d
84+
Phar::setDefaultStub(): Argument #1 ($index) must be null for a tar- or zip-based phar stub, string given
7785
string(60) "<?php // tar-based phar archive stub file
7886
__HALT_COMPILER();"

ext/phar/tests/zip/phar_setdefaultstub.phpt

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,11 @@ echo "==========================================================================
3333

3434
try {
3535
$phar->setDefaultStub('my/custom/thingy.php');
36+
} catch(Error $e) {
37+
echo $e->getMessage(). "\n";
38+
}
39+
40+
try {
3641
$phar->stopBuffering();
3742
} catch(Exception $e) {
3843
echo $e->getMessage(). "\n";
@@ -45,6 +50,11 @@ echo "==========================================================================
4550

4651
try {
4752
$phar->setDefaultStub('my/custom/thingy.php', 'the/web.php');
53+
} catch(ValueError $e) {
54+
echo $e->getMessage(). "\n";
55+
}
56+
57+
try {
4858
$phar->stopBuffering();
4959
} catch(Exception $e) {
5060
echo $e->getMessage(). "\n";
@@ -57,7 +67,7 @@ var_dump($phar->getStub());
5767
<?php
5868
unlink(__DIR__ . '/' . basename(__FILE__, '.clean.php') . '.phar.zip');
5969
?>
60-
--EXPECTF--
70+
--EXPECT--
6171
string(51) "<?php echo "Hello World\n"; __HALT_COMPILER(); ?>
6272
"
6373
============================================================================
@@ -66,13 +76,11 @@ string(60) "<?php // zip-based phar archive stub file
6676
__HALT_COMPILER();"
6777
============================================================================
6878
============================================================================
69-
70-
Warning: Phar::setDefaultStub(): Method accepts no arguments for a tar- or zip-based phar stub, 1 given in %sphar_setdefaultstub.php on line %d
79+
Phar::setDefaultStub(): Argument #1 ($index) must be null for a tar- or zip-based phar stub, string given
7180
string(60) "<?php // zip-based phar archive stub file
7281
__HALT_COMPILER();"
7382
============================================================================
7483
============================================================================
75-
76-
Warning: Phar::setDefaultStub(): Method accepts no arguments for a tar- or zip-based phar stub, 2 given in %sphar_setdefaultstub.php on line %d
84+
Phar::setDefaultStub(): Argument #1 ($index) must be null for a tar- or zip-based phar stub, string given
7785
string(60) "<?php // zip-based phar archive stub file
7886
__HALT_COMPILER();"

0 commit comments

Comments
 (0)