Frlnc / php-slack

A lightweight PHP implementation of Slack's API.
130 stars 40 forks source link

files.upload doesn't work with '@filename.ext' #6

Open slowbro opened 9 years ago

slowbro commented 9 years ago

Hello,

I found a bug with files.upload. Due to the use of http_build_query() in src/Http/CurlInteractor.php, the '@' before the filename is being urlencoded, causing cURL to not attempt a file upload. I think this can be changed to remove "http_build_query", but I have a feeling that it's there for a reason.

Is there some way this cane be worked around?

    public function post($url, array $urlParameters = [], array $postParameters = [], array $headers = [])
    {
        $request = $this->prepareRequest($url, $urlParameters, $headers);

        curl_setopt($request, CURLOPT_POST, count($postParameters));
        curl_setopt($request, CURLOPT_POSTFIELDS, http_build_query($postParameters));
        # this is the edited line, that makes the request work:
        #curl_setopt($request, CURLOPT_POSTFIELDS, $postParameters);

        return $this->executeRequest($request);
    }

the request used:

        $res = $slack->execute('files.upload', [
            'file' => '@derp.jpg',
            'filename' => 'derp.jpg',
        ]);

response using code in master:

array(3) {
  ["status_code"]=>
  int(200)
  ["headers"]=>
  array(4) {
    ["Host"]=>
    string(9) "slack.com"
    ["Accept"]=>
    string(3) "*/*"
    ["Content-Length"]=>
    string(3) "154"
    ["Content-Type"]=>
    string(33) "application/x-www-form-urlencoded"
  }
  ["body"]=>
  array(2) {
    ["ok"]=>
    bool(false)
    ["error"]=>
    string(12) "no_file_data"
  }
}

expected/response with edited code:

array(3) {
  ["status_code"]=>
  int(200)
  ["headers"]=>
  array(5) {
    ["Host"]=>
    string(9) "slack.com"
    ["Accept"]=>
    string(3) "*/*"
    ["Content-Length"]=>
    string(5) "66013"
    ["Expect"]=>
    string(12) "100-continue"
    ["Content-Type"]=>
    string(70) "multipart/form-data; boundary=----------------------------d5bae95ca2b5"
  }
  ["body"]=>
  array(2) {
    ["ok"]=>
    bool(true)
    ["file"]=>
    array(35) { ... }
  }
}
ConnorVG commented 9 years ago

I'll look into it. It looks strange that it's there on the post data.

Sent from my iPhone

On 21 Nov 2014, at 20:53, Katelyn Schiesser notifications@github.com wrote:

Hello,

I found a bug with files.upload. Due to the use of http_build_query() in src/Http/CurlInteractor.php, the '@' before the filename is being urlencoded, causing cURL to not attempt a file upload. I think this can be changed to remove "http_build_query", but I have a feeling that it's there for a reason.

Is there some way this cane be worked around?

public function post($url, array $urlParameters = [], array $postParameters = [], array $headers = [])
{
    $request = $this->prepareRequest($url, $urlParameters, $headers);

    curl_setopt($request, CURLOPT_POST, count($postParameters));
    curl_setopt($request, CURLOPT_POSTFIELDS, http_build_query($postParameters));
    # this is the edited line, that makes the request work:
    #curl_setopt($request, CURLOPT_POSTFIELDS, $postParameters);

    return $this->executeRequest($request);
}

the request used:

    $res = $slack->execute('files.upload', [
        'file' => '@derp.jpg',
        'filename' => 'derp.jpg',
    ]);

response using code in master:

array(3) { ["statuscode"]=> int(200) ["headers"]=> array(4) { ["Host"]=> string(9) "slack.com" ["Accept"]=> string(3) "/_" ["Content-Length"]=> string(3) "154" ["Content-Type"]=> string(33) "application/x-www-form-urlencoded" } ["body"]=> array(2) { ["ok"]=> bool(false) ["error"]=> string(12) "no_file_data" } } expected/response with edited code:

array(3) { ["statuscode"]=> int(200) ["headers"]=> array(5) { ["Host"]=> string(9) "slack.com" ["Accept"]=> string(3) "/_" ["Content-Length"]=> string(5) "66013" ["Expect"]=> string(12) "100-continue" ["Content-Type"]=> string(70) "multipart/form-data; boundary=----------------------------d5bae95ca2b5" } ["body"]=> array(2) { ["ok"]=> bool(true) ["file"]=> array(35) { ... } } } — Reply to this email directly or view it on GitHub.

adonix commented 7 years ago

Was this issue ☝️ ever fixed? I can't seem to upload files.

ConnorVG commented 7 years ago

I couldn't replicate the issue myself.

Are you able to replicate it consistently or is a 1-off thing?

adonix commented 7 years ago

I can consistently replicated.

ConnorVG commented 7 years ago

@adonix can you dump the entire stack trace of the error, please? Or the JSON response (if that's the case).

mychalvlcek commented 3 years ago

had the same issue, you can just simply remove http_build_query from

curl_setopt($request, CURLOPT_POSTFIELDS, http_build_query($postParameters));

by custom inherited class

class CustomCurlInteractor extends CurlInteractor {

    /**
     * {@inheritdoc}
     */
    public function post($url, array $urlParameters = [], array $postParameters = [], array $headers = [])
    {
        $request = $this->prepareRequest($url, $urlParameters, $headers);

        curl_setopt($request, CURLOPT_POST, count($postParameters));
        curl_setopt($request, CURLOPT_POSTFIELDS, $postParameters);

        return $this->executeRequest($request);
    }

}