Skip to content

http_build_query should specify arg separator #22

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.

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
weierophinney opened this issue Dec 31, 2019 · 5 comments
Closed

http_build_query should specify arg separator #22

weierophinney opened this issue Dec 31, 2019 · 5 comments

Comments

@weierophinney
Copy link
Member

In http client zendframework/zend-http/src/Client.php:1231 $body request is created using http_build_query function.

$body = http_build_query($this->getRequest()->getPost()->toArray());

Problem is that separator can be changed via ini_set('arg_separator.output', 'some other'); and this will break http client. Since "x-www-form-urlencoded" accepts only "&" as a separator, suggest to hardcode it there.

See similar issue:
https://www.drupal.org/node/2372211


Originally posted by @ameoba32 at zendframework/zend-http#33

@weierophinney
Copy link
Member Author

I had this same issue today and ended up changing the code locally to work. Calling

$client->setArgSeparator('&');
// or
$client->setArgSeparator(null);

would give me a POST body like so:

format=json&_id=561825f6d9b8689b2236d10a&note=test+note

Expected result should have been:
format=json&_id=561825f6d9b8689b2236d10a&note=test+note

Changing line #1231 to the following fixed the issue:

http_build_query($this->getRequest()->getPost()->toArray(), null, $this->getArgSeparator());
// or
http_build_query($this->getRequest()->getPost()->toArray(), null, '&');

If "x-www-form-urlencoded" accepts only "&" as a separator (as ameoba32 suggests), then this fix is not the best and "&" should be hard-coded (in case someone has arg_separator.output set in their php.ini file).

This issue has existed in this file for quite some time (as I had to make the change on another project over a year ago). It seems like a simple fix...can we do it?


Originally posted by @redheadedstep at zendframework/zend-http#33 (comment)

@weierophinney
Copy link
Member Author

@redheadedstep Are you willing to provide a pull request with tests for the behavior? I'd certainly review...


Originally posted by @weierophinney at zendframework/zend-http#33 (comment)

@weierophinney
Copy link
Member Author

@weierophinney , there is one already zendframework/zend-http#34


Originally posted by @ameoba32 at zendframework/zend-http#33 (comment)

@weierophinney
Copy link
Member Author

yes, I saw that pull request.

at the moment, I am manually updating my copy of the code, but any new updates overwrite my changes and give me at least a few minutes of wondering why all my curl requests are failing with parameters being passed with & like below :-P

[Thu Mar 31 05:06:29 2016] [error] FavoriteAction::executePost :: array (
  'module' => 'account',
  'action' => 'favorite',
  'format' => 'json',
  'amp;store' => '55efae4cd9b868293e8b4583',
)

Here is what I have in my copy of Client...which is what pull request #34 does.

http_build_query($this->getRequest()->getPost()->toArray(), null, '&');

Now I'm just waiting for merge for that request #34 that was committed last December. Any love on the ETA?


Originally posted by @redheadedstep at zendframework/zend-http#33 (comment)

@samsonasik
Copy link
Member

it seems already fixed by zendframework/zend-http#34 . I am closing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants