Skip to content
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

Add support for parameters to GET requests #12

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

danhunsaker
Copy link
Collaborator

Addresses GitHub Issue #8

  • Set parameters as CURLOPT_POSTFIELDS, which cURL will automatically translate into the URL query string.
    This lets us pass the ?params versus &params logic off to cURL, which makes the code easier to maintain.
  • Allow all parameters passed to be empty by making the argument optional on GET, POST, and PUT.
    This is mostly for consistency.

Signed-off-by: Daniel Hunsaker [email protected]

@danhunsaker
Copy link
Collaborator Author

With the latest commit, this has been tested and functions properly. WIll wait a few days for feedback, then merge if nobody has any concerns.

@naja7host
Copy link

i will check and test it tonight :)

// longer, so we need to build them into the query string ourselves.
$action_postfields_string = http_build_query($put_post_parameters);
$action_path .= (strpos($action_path, '?') === FALSE ? '?' : '&') . $action_postfields_string;
unset($action_postfields_string);
Copy link
Owner

Choose a reason for hiding this comment

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

love the use of unset :) memory savings ftw

@naja7host
Copy link

it not working , because the class send the first request $action_path before the the switch case .

so the get parametre is set later , and it should build with the $action_path before it sended .

@naja7host
Copy link

moving the line

curl_setopt($prox_ch, CURLOPT_URL, "https://{$this->hostname}:{$this->port}/api2/json{$action_path}");

to the end of switch case , do the trick .

@danhunsaker
Copy link
Collaborator Author

Sorry for the delay on this one. I made these adjustments to the code and thought I'd pushed them, then I lost Internet access until just a few minutes ago, so I just now saw that the changes weren't pushed after all. Go ahead and give it another go now.

@naja7host
Copy link

this is still not working ...

anyone has tested this ?

Addresses GitHub Issue CpuID#8

- Set parameters as CURLOPT_POSTFIELDS, which cURL will automatically translate into the URL query string.
  This lets us pass the ?params versus &params logic off to cURL, which makes the code easier to maintain.
- Allow all parameters passed to be empty by making the argument optional on GET, POST, and PUT.
  This is mostly for consistency.

Signed-off-by: Daniel Hunsaker <[email protected]>
Addresses GitHub issue CpuID#8

- cURL appears to not (or at least to no longer) actually support turning POSTFIELDS into query string parameters,
  so we have to build the query string ourselves.  This update does just that.

Signed-off-by: Daniel Hunsaker <[email protected]>
@danhunsaker danhunsaker force-pushed the bugfix/support-get-params branch from 4ee7d36 to 0adb4a7 Compare July 14, 2016 22:50
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

Successfully merging this pull request may close these issues.

3 participants