Skip to content

Added a note about no body for GET requests #131

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

Merged
merged 5 commits into from
Aug 2, 2016

Conversation

Nyholm
Copy link
Member

@Nyholm Nyholm commented Jul 28, 2016

Writing this in the documentation makes us more serious.


This adapter violates the Liskov substitution principle in a rare edge case. When the adapter is configured to use
Buzz' Curl client, it does not send request bodies for request methods such as GET, HEAD and TRACE. A RequestException
will be thrown if this ever happens.
Copy link
Contributor

@dbu dbu Aug 2, 2016

Choose a reason for hiding this comment

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

lets add: "The only solution if you need GET requests with a body (e.g. for Elasticsearch) is to use [whatever] in Buzz or a different client like Guzzle 6.

@Nyholm
Copy link
Member Author

Nyholm commented Aug 2, 2016

The PR is updated.

There is no workaround for this limitation BTW. It is a limitation in Buzz.

@dbu
Copy link
Contributor

dbu commented Aug 2, 2016

doesn't buzz have other clients than curl?

it looks like you need to add Elasticsearch to the dictionary.

@Nyholm
Copy link
Member Author

Nyholm commented Aug 2, 2016

doesn't buzz have other clients than curl?

True.

The PR is updated

@dbu dbu merged commit 9970840 into php-http:master Aug 2, 2016
@dbu
Copy link
Contributor

dbu commented Aug 2, 2016

👍

@Nyholm
Copy link
Member Author

Nyholm commented Aug 2, 2016

Thank you for merging

@Nyholm Nyholm deleted the buzz-warning branch August 2, 2016 15:55
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.

2 participants