php-http / message

HTTP Message related tools
http://php-http.org
MIT License
1.3k stars 42 forks source link

Unable to format binary payload as command #91

Closed z38 closed 6 years ago

z38 commented 6 years ago
Q A
Bug? yes
New Feature? no
Version master (977edb5)

Actual Behavior

Using CurlCommandFormatter to format a HTTP request with a binary payload throws an exception:

Error: escapeshellarg(): Input string contains NULL bytes
  at vendor/php-http/message/src/Formatter/CurlCommandFormatter.php:43

Expected Behavior

The formatter throws a proper exception or outputs a valid (possibly incomplete) command (see below).

Steps to Reproduce

$requestFactory = Http\Discovery\MessageFactoryDiscovery::find();
$request = $requestFactory->createRequest('POST', 'http://example.com', [], "\0");

$formatter = new Http\Message\Formatter\CurlCommandFormatter();
$formatter->formatRequest($request);

Possible Solutions

In case the formatter should output a command at any times, it should either escape null bytes properly or remove the payload completely. Other tools, such as the network monitor of Firefox, only include url-encoded payloads or multipart parts with Content-Disposition: form-data in the command.

dbu commented 6 years ago

is \0 the only thing that has a problem on the cli? what about other control characters, are they all handled by escapeshellarg? if you want to work on a pull request, i guess a first step would be to detect if the stream won't work and then skipping the curl command completely, or outputting something with a body of "original binary stream can not be rendered" or somesuch?

michaeldk commented 6 years ago

Any idea when this will be officially released?

Nyholm commented 6 years ago

I do not mind releasing a patch release with this PR now.

It is just code style and docs in dev-master except for this PR.

michaeldk commented 6 years ago

Hey :)

I thought you might receive a notification from my "Like" but I guess not.

I'd be pleased to have this fix released so I can keep working on my issue :)

dbu commented 6 years ago

@Nyholm do you want to wait for #89 ? as far as i can tell #89 is not a fix for a regression, so we could also release a patch version now and then another patch version once 89 is in?