Auto-adding content-length heading to fix append-command bug #21
Conversation
src/org/apache/hadoop/tools/Curl.php
Outdated
| $length = 0; | ||
| if (isset($options[CURLOPT_POSTFIELDS])) { | ||
| if (function_exists('mb_strlen')) { | ||
| $length = mb_strlen($options[CURLOPT_POSTFIELDS]); |
There was a problem hiding this comment.
The Content-Length HTTP header should be a pure binary character length of the body, aka raw bytes, not a human readable length. The former would be always calculated using strlen, and the latter would be mb_strlen. Let's use strlen always here.
Source: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Length
src/org/apache/hadoop/tools/Curl.php
Outdated
| $length = strlen($options[CURLOPT_POSTFIELDS]); | ||
| } | ||
| } | ||
| $options[CURLOPT_HTTPHEADER] = array_merge($options[CURLOPT_HTTPHEADER], ['Content-Length: '.$length]); |
There was a problem hiding this comment.
nit: spacing around . char? Not sure what the code style of this repo is though.
There was a problem hiding this comment.
yup, I missed it. thanks!
src/org/apache/hadoop/tools/Curl.php
Outdated
| if (!$has_content_length_header && !isset($options[CURLOPT_INFILE]) && !isset($options[CURLOPT_INFILESIZE])) { | ||
| $length = 0; | ||
| if (isset($options[CURLOPT_POSTFIELDS])) { | ||
| $length = strlen($options[CURLOPT_POSTFIELDS]); |
There was a problem hiding this comment.
actually one more possible caveat; it looks like CURLOPT_POSTFIELDS could be a @ prefixed string which implies a file name that is then read in (deprecated in PHP 5.5 though) or it could even be a key/value array, on which strlen will fail. To be safe better also check if is_string and that the string does not begin with @.
There was a problem hiding this comment.
Oh yes, this library is >= php5.4.0 ... will make that change now
Sending an
APPENDcommand is a two-step process. The first POST request is sent withContent-Length: -1header, which returns in aBad Requestresponse. Setting it to0fixes this issue.