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.

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

Jump to bottom

Add proper Content-Length based off of the request body's length #85

Merged
merged 1 commit into from
Jul 3, 2013
Merged

Add proper Content-Length based off of the request body's length #85

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
3 changes: 3 additions & 0 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,8 @@
"psr-0": {
"Httpful": "src/"
}
},
"require-dev": {
"phpunit/phpunit": "*"
}
}
15 changes: 9 additions & 6 deletions src/Httpful/Request.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -784,6 +784,14 @@ public function _curlPrep()
curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, $this->strict_ssl);
curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);

// https://github.com/nategood/httpful/issues/84
// set Content-Length to the size of the payload if present
if (isset($this->payload)) {
$this->serialized_payload = $this->_serializePayload($this->payload);
curl_setopt($ch, CURLOPT_POSTFIELDS, $this->serialized_payload);
$this->headers['Content-Length'] = strlen($this->serialized_payload);
Copy link
Owner

Choose a reason for hiding this comment

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

Haven't checked the HTTP spec yet, but should this be multi byte string length mb_strlen?

Copy link
Owner

Choose a reason for hiding this comment

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

Nope, I think you are correct. We want strlen even for multibyte characters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct. It is the size in bytes not number of characters.
On Jul 3, 2013 12:00 PM, "Nate Good" [email protected] wrote:

In src/Httpful/Request.php:

@@ -784,6 +784,14 @@ public function _curlPrep()
curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, $this->strict_ssl);
curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);

  •    // https://github.com/nategood/httpful/issues/84

  •    // set Content-Length to the size of the payload if present

  •    if (isset($this->payload)) {

  •        $this->serialized_payload = $this->_serializePayload($this->payload);

  •        curl_setopt($ch, CURLOPT_POSTFIELDS, $this->serialized_payload);

  •        $this->headers['Content-Length'] = strlen($this->serialized_payload);

Nope, I think you are correct. We want strlen even for multibyte
characters.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/85/files#r5012797
.

Copy link
Owner

Choose a reason for hiding this comment

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

Yep. Silly question. Brain is moving slow this morning.

Copy link

Choose a reason for hiding this comment

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

Result:
string 'test' (length=4)
int 4
int 4
int 4
string 'тест' (length=8)
int 4
int 4
int 8

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That fix is incorrect:
http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.13

More specifically: Applications SHOULD use this field to indicate the
transfer-length of the message-body, unless this is prohibited by the rules
in section 4.4.

On Fri, Sep 13, 2013 at 11:12 AM, wackyfrog [email protected]:

In src/Httpful/Request.php:

@@ -784,6 +784,14 @@ public function _curlPrep()
curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, $this->strict_ssl);
curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);

  •    // https://github.com/nategood/httpful/issues/84

  •    // set Content-Length to the size of the payload if present

  •    if (isset($this->payload)) {

  •        $this->serialized_payload = $this->_serializePayload($this->payload);

  •        curl_setopt($ch, CURLOPT_POSTFIELDS, $this->serialized_payload);

  •        $this->headers['Content-Length'] = strlen($this->serialized_payload);

Result:
string 'test' (length=4)
int 4
int 4
int 4
string 'тест' (length=8)
int 4
int 4
int 8

—

Reply to this email directly or view it on GitHubhttps://github.com//pull/85/files#r6349942
.

}

$headers = array();
// https://github.com/nategood/httpful/issues/37
// Except header removes any HTTP 1.1 Continue from response headers
Expand All @@ -807,7 +815,7 @@ public function _curlPrep()
$headers[] = $accept;
}

//Solve a bug on squid proxy, NONE/411 when miss content length
// Solve a bug on squid proxy, NONE/411 when miss content length
if (!isset($this->headers['Content-Length'])) {
$this->headers['Content-Length'] = 0;
}
Expand All @@ -826,11 +834,6 @@ public function _curlPrep()

curl_setopt($ch, CURLOPT_HTTPHEADER, $headers);

if (isset($this->payload)) {
$this->serialized_payload = $this->_serializePayload($this->payload);
curl_setopt($ch, CURLOPT_POSTFIELDS, $this->serialized_payload);
}

if ($this->_debug) {
curl_setopt($ch, CURLOPT_VERBOSE, true);
}
Expand Down