thephpleague / oauth1-client

OAuth 1 Client
MIT License
968 stars 73 forks source link

Multidimensional arrays #24

Closed jenssegers closed 9 years ago

jenssegers commented 9 years ago

When sending multidimensional array data through the client, you will get this error:

rawurlencode() expects parameter 1 to be string, array given

Is this a bug, or is this spec related?

bencorlett commented 9 years ago

Can you please provide more information?

jenssegers commented 9 years ago

Sorry, I was talking about the signature generation: https://github.com/thephpleague/oauth1-client/blob/master/src/Client/Signature/HmacSha1Signature.php#L66

For example; if you send something like this:

[
    'foo' => 'bar',
    'data' => [
        'foo' => 'bar'
    ]
]

It will try to run rawurlencode on the data array and throw that error.

I tried replacing it with http_build_query($data, '', '&', PHP_QUERY_RFC3986), but that filters out null values which is not really what it is supposed to do.

jenssegers commented 9 years ago

You can replicate this with this adjusted unit test:

public function testSigningRequest()
{
    $signature = new HmacSha1Signature($this->getMockClientCredentials());

    $uri = 'http://www.example.com/?qux=corge';
    $parameters = array('foo' => 'bar', 'baz' => null, 'data' => ['foo' => 'bar']);

    $this->assertEquals('...', $signature->sign($uri, $parameters));
}
bencorlett commented 9 years ago

Hmm, from what I can remember the reason I did that was related to the spec.

Have you got an actual use-case for needing nesting or is it a hypothetical edge-case you’ve discovered?

On 11 May 2015, at 8:12 pm, Jens Segers notifications@github.com wrote:

You can replicate this with this adjusted unit test:

public function testSigningRequest() { $signature = new HmacSha1Signature($this->getMockClientCredentials());

$uri = 'http://www.example.com/?qux=corge';
$parameters = array('foo' => 'bar', 'baz' => null, 'data' => ['foo' => 'bar']);

$this->assertEquals('A3Y7C1SUHXR1EBYIUlT3d6QT1cQ=', $signature->sign($uri, $parameters));

} — Reply to this email directly or view it on GitHub https://github.com/thephpleague/oauth1-client/issues/24#issuecomment-100846370.

jenssegers commented 9 years ago

No this is an actual use case, but for an internal company API.

bencorlett commented 9 years ago

I did some reading around and saw some implementations over at php.net, but nothing that I really liked.

Are you able to see if something like this would work for you? Maybe if it does, you could integrate that code and shoot through a PR with the offending unit test passing?

jenssegers commented 9 years ago

I tested Python's oauthlib and it turns out it has the same issue. Is this use case just not common, or is it against the oauth1 spec or something?

The fix that I used is:

$data = array_merge($query, $parameters);

// http_build_query messes up null and boolean values, so we need to convert
// them before we run the function on the data.
array_walk_recursive($data, function (&$value) {
    if ($value === null or is_bool($value)) {
        $value = rawurlencode($value);
    }
});

ksort($data);

$baseString .= rawurlencode(http_build_query($data, '', '&', PHP_QUERY_RFC3986));

But in this case, ksort would only sort on the first level, and not on secondary levels (but I have no idea if that is required by the spec).

jenssegers commented 9 years ago

I just found http://oauth.googlecode.com/svn/code/javascript/example/signature.html. It seems that it takes the "data[foo]" string and encodes it similar to the drupal example before sorting.

Parameters:

test[foo]=foo&test[bar]=bar

Result:

test%5Bbar%5D=bar&test%5Bfoo%5D=foo
bencorlett commented 9 years ago

The spec just mentions that values need to be encoded against some RFC that rawurlencode adheres to. It's unique to Oauth 1 as Oauth 2 doesn't require we generate signatures and things, it assumes SSL or TLS is used for encryption.

Is there a chance you can change your internal API? I'm hesitant to move away incase we are not adhering to that RFC if we implement a change. The goal of this library is to be 100% spec compliant, which it currently is :)

Sent from my iPhone

Please excuse my brevity

On 12 May 2015, at 8:32 pm, Jens Segers notifications@github.com wrote:

I tested Python's oauthlib and it turns out it has the same issue. Is this use case just not common, or is it against the oauth1 spec or something?

The fix that I used is:

$data = array_merge($query, $parameters);

// http_build_query messes up null and boolean values, so we need to convert // them before we run the function on the data. array_walk_recursive($data, function (&$value) { if ($value === null or is_bool($value)) { $value = rawurlencode($value); } });

ksort($data);

$baseString .= rawurlencode(http_build_query($data, '', '&', PHP_QUERY_RFC3986)); — Reply to this email directly or view it on GitHub.

bencorlett commented 9 years ago

Please reopen the issue if you'd like to discuss further :)