Skip to content

Added put, patch and delete request_methods plus tests. #1519

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Added put, patch and delete request_methods plus tests. #1519

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 6 additions & 1 deletion main/SAPI.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,12 @@ SAPI_API void sapi_activate(void)
if (PG(enable_post_data_reading)
&& SG(request_info).content_type
&& SG(request_info).request_method
&& !strcmp(SG(request_info).request_method, "POST")) {
&&
(!strcmp(SG(request_info).request_method, "POST") ||
!strcmp(SG(request_info).request_method, "PUT") ||
!strcmp(SG(request_info).request_method, "PATCH") ||
!strcmp(SG(request_info).request_method, "DELETE"))
) {
/* HTTP POST may contain form data to be processed into variables
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment needs minor update, as it only/explicitly mentions POST

* depending on given content type */
sapi_read_post_data();
Expand Down
8 changes: 7 additions & 1 deletion main/php_content_types.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,13 @@ static sapi_post_entry php_post_entries[] = {
*/
SAPI_API SAPI_POST_READER_FUNC(php_default_post_reader)
{
if (!strcmp(SG(request_info).request_method, "POST")) {
if (
!strcmp(SG(request_info).request_method, "POST") ||
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be && instead of ||?

!strcmp(SG(request_info).request_method, "PUT") ||
!strcmp(SG(request_info).request_method, "PATCH") ||
!strcmp(SG(request_info).request_method, "DELETE")
) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mixed tabs and spaces here.


if (NULL == SG(request_info).post_entry) {
/* no post handler registered, so we just swallow the data */
sapi_read_standard_form_data();
Expand Down
6 changes: 5 additions & 1 deletion main/php_variables.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,11 @@ static zend_bool php_auto_globals_create_post(zend_string *name)
(strchr(PG(variables_order),'P') || strchr(PG(variables_order),'p')) &&
!SG(headers_sent) &&
SG(request_info).request_method &&
!strcasecmp(SG(request_info).request_method, "POST")) {
(!strcasecmp(SG(request_info).request_method, "POST") ||
!strcasecmp(SG(request_info).request_method, "PUT") ||
!strcasecmp(SG(request_info).request_method, "PATCH") ||
!strcasecmp(SG(request_info).request_method, "DELETE"))
) {
sapi_module.treat_data(PARSE_POST, NULL, NULL);
} else {
zval_ptr_dtor(&PG(http_globals)[TRACK_VARS_POST]);
Expand Down
92 changes: 86 additions & 6 deletions run-tests.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1296,7 +1296,7 @@ function run_test($php, $file, $env)
unset($section_text['FILEEOF']);
}

foreach (array( 'FILE', 'EXPECT', 'EXPECTF', 'EXPECTREGEX' ) as $prefix) {
foreach (array( 'FILE', 'EXPECT', 'EXPECTF', 'EXPECTREGEX' ) as $prefix) {
$key = $prefix . '_EXTERNAL';

if (@count($section_text[$key]) == 1) {
Expand Down Expand Up @@ -1340,8 +1340,8 @@ function run_test($php, $file, $env)

$tested = trim($section_text['TEST']);

/* For GET/POST/PUT tests, check if cgi sapi is available and if it is, use it. */
if (!empty($section_text['GET']) || !empty($section_text['POST']) || !empty($section_text['GZIP_POST']) || !empty($section_text['DEFLATE_POST']) || !empty($section_text['POST_RAW']) || !empty($section_text['PUT']) || !empty($section_text['COOKIE']) || !empty($section_text['EXPECTHEADERS'])) {
/* For GET/POST/PUT/PATCH/DELETE tests, check if cgi sapi is available and if it is, use it. */
if (!empty($section_text['GET']) || !empty($section_text['POST']) || !empty($section_text['GZIP_POST']) || !empty($section_text['DEFLATE_POST']) || !empty($section_text['POST_RAW']) || !empty($section_text['PUT']) || !empty($section_text['PATCH']) || !empty($section_text['DELETE']) || !empty($section_text['COOKIE']) || !empty($section_text['EXPECTHEADERS'])) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have POST, GZIP_POST, DEFLATE_POST, POST_RAW, why only PUT, PATCH and DELETE? I have no idea what this line does, it's just an inconsistency I saw.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure we should add all this test facilities for all request-methods. I would add those only when we actually have tests which require them

if (isset($php_cgi)) {
$old_php = $php;
$php = $php_cgi . ' -C ';
Expand Down Expand Up @@ -1594,9 +1594,9 @@ function run_test($php, $file, $env)
}
}
}

if (!extension_loaded("zlib")
&& ( array_key_exists("GZIP_POST", $section_text)
&& ( array_key_exists("GZIP_POST", $section_text)
|| array_key_exists("DEFLATE_POST", $section_text))
) {
$message = "ext/zlib required";
Expand Down Expand Up @@ -1770,6 +1770,10 @@ function run_test($php, $file, $env)
$request .= $line;
}

if (empty($env['CONTENT_TYPE'])) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a unrelated change in old POST logic?

$env['CONTENT_TYPE'] = 'application/x-www-form-urlencoded';
}

$env['CONTENT_LENGTH'] = strlen($request);
$env['REQUEST_METHOD'] = 'PUT';

Expand All @@ -1781,6 +1785,82 @@ function run_test($php, $file, $env)
save_text($tmp_post, $request);
$cmd = "$php $pass_options $ini_settings -f \"$test_file\" 2>&1 < \"$tmp_post\"";

} elseif (array_key_exists('PATCH', $section_text) && !empty($section_text['PATCH'])) {

$post = trim($section_text['PATCH']);
$raw_lines = explode("\n", $post);

$request = '';
$started = false;

foreach ($raw_lines as $line) {

if (empty($env['CONTENT_TYPE']) && preg_match('/^Content-Type:(.*)/i', $line, $res)) {
$env['CONTENT_TYPE'] = trim(str_replace("\r", '', $res[1]));
continue;
}

if ($started) {
$request .= "\n";
}

$started = true;
$request .= $line;
}

if (empty($env['CONTENT_TYPE'])) {
$env['CONTENT_TYPE'] = 'application/x-www-form-urlencoded';
}

$env['CONTENT_LENGTH'] = strlen($request);
$env['REQUEST_METHOD'] = 'PATCH';

if (empty($request)) {
junit_mark_test_as('BORK', $shortname, $tested, null, 'empty $request');
return 'BORKED';
}

save_text($tmp_post, $request);
$cmd = "$php $pass_options $ini_settings -f \"$test_file\" 2>&1 < \"$tmp_post\"";

} elseif (array_key_exists('DELETE', $section_text) && !empty($section_text['DELETE'])) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesnt it require a similar branch for PUT?


$post = trim($section_text['DELETE']);
$raw_lines = explode("\n", $post);

$request = '';
$started = false;

foreach ($raw_lines as $line) {

if (empty($env['CONTENT_TYPE']) && preg_match('/^Content-Type:(.*)/i', $line, $res)) {
$env['CONTENT_TYPE'] = trim(str_replace("\r", '', $res[1]));
continue;
}

if ($started) {
$request .= "\n";
}

$started = true;
$request .= $line;
}

if (empty($env['CONTENT_TYPE'])) {
$env['CONTENT_TYPE'] = 'application/x-www-form-urlencoded';
}

$env['CONTENT_LENGTH'] = strlen($request);
$env['REQUEST_METHOD'] = 'DELETE';

if (empty($request)) {
junit_mark_test_as('BORK', $shortname, $tested, null, 'empty $request');
return 'BORKED';
}

save_text($tmp_post, $request);
$cmd = "$php $pass_options $ini_settings -f \"$test_file\" 2>&1 < \"$tmp_post\"";

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a lot of duplicated code here.

} else if (array_key_exists('POST', $section_text) && !empty($section_text['POST'])) {

$post = trim($section_text['POST']);
Expand Down Expand Up @@ -2185,7 +2265,7 @@ function run_test($php, $file, $env)
if (isset($old_php)) {
$php = $old_php;
}

$diff = empty($diff) ? '' : preg_replace('/\e/', '<esc>', $diff);

junit_mark_test_as($restype, str_replace($cwd . '/', '', $tested_file), $tested, null, $info, $diff);
Expand Down
9 changes: 9 additions & 0 deletions tests/basic/033.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
--TEST--
Simple PUT Method test
--PUT--
a=Hello+World
--FILE--
<?php
echo $_POST['a']; ?>
--EXPECT--
Hello World
9 changes: 9 additions & 0 deletions tests/basic/034.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
--TEST--
Simple PATCH Method test
--PATCH--
a=Hello+World
--FILE--
<?php
echo $_POST['a']; ?>
--EXPECT--
Hello World
9 changes: 9 additions & 0 deletions tests/basic/035.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
--TEST--
Simple DELETE Method test
--DELETE--
a=Hello+World
--FILE--
<?php
echo $_POST['a']; ?>
--EXPECT--
Hello World