thephpleague / oauth1-client

OAuth 1 Client
MIT License
968 stars 73 forks source link

Support query string and request parameters with arrays #42

Closed nicktacular closed 8 years ago

nicktacular commented 8 years ago

Implemented a fix to support queries or request parameters that contain associative arrays or indices. For example, if trying to sign a URL like https://example.org/hello?what[]=1&what[]=2&is[][second]=hello&is[][second]=this then the base string generation fails because an array is encountered. Furthermore, this implementation does not use the parse_url method because it does it incorrectly with respect to how we need to actually sign the end request.

Fixes #39

bencorlett commented 8 years ago

Hi Nick,

I'm going to just sit on this for a little, I remember attacking this problem with a similar approach to you, but there was a reason I didn't. I think the OAuth spec mentions something like not having nested parameters however I can't remember why.

In short, I need to do a bit of reading to make sure we're not diverting away from the OAuth spec as that's the USP of this package :)

@stevenmaguire what are your thoughts?

stevenmaguire commented 8 years ago

Before discussing the solution too much, I'd like to better understand the nature of the issue a bit more. I've reviewed the spec and it does not explicitly say NOT to use array values in query string parameters and it does not explicitly say they are ok, nor do they provide examples. I will provide some follow-up in the issue #39.

nicktacular commented 8 years ago

Thanks for taking a look, @bencorlett, @stevenmaguire.

I agree that the spec is very vague on what should be done here. Unfortunately, I've not found how other APIs deal with this but I have one such example which I'm building a client against which treats signing of parameters in this manner: https://oauth-api.beatport.com/

Now, by no means is that necessarily the correct way but regardless of how the server implements this, I think that the tool should probably not issue a warning when an unexpected array is present.

Perhaps the solution here is to provide the signature with a closure or implementation that follows some specific interface on how to solve this problem, so that if there's another API that does it another way, then an implementation can be written and injected into the signer.

Thoughts?

stevenmaguire commented 8 years ago

@nicktacular I've posed a couple questions in issue #39. Could you respond to those within the issue conversation? I'd like to better understand the problem before discussing the solution design.

nicktacular commented 8 years ago

Unfortunately at this time I don't have the capacity to pursue this.