Skip to content

Implemented FR #51634: Can't post multiple fields with the same name #8292

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

Implemented FR #51634: Can't post multiple fields with the same name #8292

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
85 changes: 58 additions & 27 deletions ext/curl/interface.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1991,6 +1991,54 @@ static void free_cb(void *arg) /* {{{ */
/* }}} */
#endif

static inline CURLcode add_simple_field(curl_mime *mime, zend_string *string_key, zval *current)
{
CURLcode error = CURLE_OK;
#if LIBCURL_VERSION_NUM >= 0x073800 /* 7.56.0 */
curl_mimepart *part;
CURLcode form_error;
#else
struct HttpPost *first = NULL;
struct HttpPost *last = NULL;
CURLFORMcode form_error;
#endif
zend_string *postval, *tmp_postval;

postval = zval_get_tmp_string(current, &tmp_postval);

#if LIBCURL_VERSION_NUM >= 0x073800 /* 7.56.0 */
part = curl_mime_addpart(mime);
if (part == NULL) {
zend_tmp_string_release(tmp_postval);
zend_string_release_ex(string_key, 0);
return CURLE_OUT_OF_MEMORY;
}
if ((form_error = curl_mime_name(part, ZSTR_VAL(string_key))) != CURLE_OK
|| (form_error = curl_mime_data(part, ZSTR_VAL(postval), ZSTR_LEN(postval))) != CURLE_OK) {
error = form_error;
}
#else
/* The arguments after _NAMELENGTH and _CONTENTSLENGTH
* must be explicitly cast to long in curl_formadd
* use since curl needs a long not an int. */
form_error = curl_formadd(&first, &last,
CURLFORM_COPYNAME, ZSTR_VAL(string_key),
CURLFORM_NAMELENGTH, ZSTR_LEN(string_key),
CURLFORM_COPYCONTENTS, ZSTR_VAL(postval),
CURLFORM_CONTENTSLENGTH, ZSTR_LEN(postval),
CURLFORM_END
);

if (form_error != CURL_FORMADD_OK) {
/* Not nice to convert between enums but we only have place for one error type */
error = (CURLcode)form_error;
}
#endif
zend_tmp_string_release(tmp_postval);

return error;
}

static inline zend_result build_mime_structure_from_hash(php_curl *ch, zval *zpostfields) /* {{{ */
{
HashTable *postfields = Z_ARRVAL_P(zpostfields);
Expand Down Expand Up @@ -2018,7 +2066,7 @@ static inline zend_result build_mime_structure_from_hash(php_curl *ch, zval *zpo
#endif

ZEND_HASH_FOREACH_KEY_VAL(postfields, num_key, string_key, current) {
zend_string *postval, *tmp_postval;
zend_string *postval;
/* Pretend we have a string_key here */
if (!string_key) {
string_key = zend_long_to_str(num_key);
Expand Down Expand Up @@ -2181,36 +2229,19 @@ static inline zend_result build_mime_structure_from_hash(php_curl *ch, zval *zpo
continue;
}

postval = zval_get_tmp_string(current, &tmp_postval);
if (Z_TYPE_P(current) == IS_ARRAY) {
zval *current_element;

ZEND_HASH_FOREACH_VAL(HASH_OF(current), current_element) {
add_simple_field(mime, string_key, current_element);
} ZEND_HASH_FOREACH_END();

#if LIBCURL_VERSION_NUM >= 0x073800 /* 7.56.0 */
part = curl_mime_addpart(mime);
if (part == NULL) {
zend_tmp_string_release(tmp_postval);
zend_string_release_ex(string_key, 0);
return FAILURE;
}
if ((form_error = curl_mime_name(part, ZSTR_VAL(string_key))) != CURLE_OK
|| (form_error = curl_mime_data(part, ZSTR_VAL(postval), ZSTR_LEN(postval))) != CURLE_OK) {
error = form_error;
continue;
}
#else
/* The arguments after _NAMELENGTH and _CONTENTSLENGTH
* must be explicitly cast to long in curl_formadd
* use since curl needs a long not an int. */
form_error = curl_formadd(&first, &last,
CURLFORM_COPYNAME, ZSTR_VAL(string_key),
CURLFORM_NAMELENGTH, ZSTR_LEN(string_key),
CURLFORM_COPYCONTENTS, ZSTR_VAL(postval),
CURLFORM_CONTENTSLENGTH, ZSTR_LEN(postval),
CURLFORM_END);

if (form_error != CURL_FORMADD_OK) {
/* Not nice to convert between enums but we only have place for one error type */
error = (CURLcode)form_error;
}
#endif
zend_tmp_string_release(tmp_postval);
add_simple_field(mime, string_key, current);
Copy link
Member

Choose a reason for hiding this comment

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

Nit: An else-case and removing the continue could remove the duplicate zend_string_release_ex.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but IMO the current code is more readable, as each special block now looks the same. If we'd to add one in the future, then it's less likely the wrong thing gets copied. (I'm also allergic to else).


zend_string_release_ex(string_key, 0);
} ZEND_HASH_FOREACH_END();

Expand Down
63 changes: 63 additions & 0 deletions ext/curl/tests/curl_postfields_array.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
--TEST--
CURLOPT_POSTFIELDS with multi-value fields
--EXTENSIONS--
sockets
--FILE--
<?php
$socket = stream_socket_server("tcp://0.0.0.0:29999", $errno, $errstr);

if (!$socket) {
echo "$errstr ($errno)<br />\n";
return;
}

$url = "http://127.0.0.1:29999/get.inc?test=raw";

$fields = [
'single' => 'SingleValue',
'multi' => [
'Multi1',
'Multi2',
]
];

$options = array(
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason not to use the short array syntax [] here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not really, I probably copied it from the original example. Will fix.

CURLOPT_POST => 1,
CURLOPT_HEADER => 0,
CURLOPT_URL => $url,
CURLOPT_FRESH_CONNECT => 1,
CURLOPT_RETURNTRANSFER => 1,
CURLOPT_FORBID_REUSE => 1,
CURLOPT_TIMEOUT => 1,
CURLOPT_POSTFIELDS => $fields,
);

$ch = curl_init();
curl_setopt_array($ch, $options);

$curl_content = curl_exec($ch);
curl_close($ch);

$conn = stream_socket_accept($socket);
echo stream_get_contents($conn);
?>
--EXPECTF--
POST /get.inc?test=raw HTTP/1.1
Host: %s
Accept: */*
Content-Length: %d
Content-Type: multipart/form-data; boundary=------------------------%s

--------------------------%s
Content-Disposition: form-data; name="single"

SingleValue
--------------------------%s
Content-Disposition: form-data; name="multi"
Copy link

Choose a reason for hiding this comment

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

@derickr Hi! Sorry to bother you. I am trying to do the same request with PHP 8.2.9, which doesn't work as expected. What I get is that names are overwritten. So, from this particular test, the receiver will only get Multi2 value. Shouldn't the name be name="multi[]" ?

Copy link
Member

Choose a reason for hiding this comment

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

No, multi is correct; but like the feature description states:

This server can't be PHP, because PHP's POST processing does not allow for two[sic] the same field names.


Multi1
--------------------------%s
Content-Disposition: form-data; name="multi"

Multi2
--------------------------%s--