Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

http_build_query should specify arg separator #33

Open
ameoba32 opened this issue Dec 3, 2015 · 5 comments
Open

http_build_query should specify arg separator #33

ameoba32 opened this issue Dec 3, 2015 · 5 comments

Comments

@ameoba32
Copy link
Contributor

ameoba32 commented Dec 3, 2015

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

@redheadedstep
Copy link

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?

@weierophinney
Copy link
Member

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

@ameoba32
Copy link
Contributor Author

@weierophinney , there is one already #34

@redheadedstep
Copy link

redheadedstep commented Apr 15, 2016

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?

@weierophinney
Copy link
Member

This repository has been closed and moved to laminas/laminas-http; a new issue has been opened at laminas/laminas-http#22.

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

No branches or pull requests

3 participants