Base string incorrect for array type GET query string about oauth-1.0a

[dinoboff]

IMHO, OAuth-1.0a is a low level library and should not try to encode complex object; there's many way to encode them and there's no way to ensure the http client encoding will match OAuth-1.0a one. The solution would be to encode the data first, or let the http client encode it, and provide the encoded data to OAuth-1.0a. OAuth-1.0a would have to parse it like it parses the query string.

An alternative would be to add a data_encoder option to override the default simple encoder.

ps: probably encoder is not right word.

[robmacecs]

Also just in case anyone was wondering, I've removed the oauth_ params from the parameter string, but they were there in the correct place

[ddo]

will check

[robmacecs]

Okay so, I've had a bit of a dig around. The problem seems to arise here:

base_string_data = this.sortObject(this.percentEncodeData(this.mergeObject(oauth_data, this.mergeObject(request.data, this.deParamUrl(request.url)))));

If I log base_string_data at this point it looks like this

[
  { key: 'filter', value: [ '%5Bobject%20Object%5D' ] },
  { key: 'filter%255B0%255D%255Battribute%255D', value: 'updated_at' },
  { key: 'filter%255B0%255D%255Bfrom%255D', value: '2017-05-10%2000%3A00%3A00' },
  { key: 'limit', value: '100' },
  // oauth_* values
  { key: 'page', value: '1' }
]

So you can see, that filter has become 3 separate keys. I'm not sure how exactly this would best be fixed but I'll keep looking into it for now. It seems there needs to be an additional check in percentEncodeData in order to avoid this but if I find anything I'll update this

[robmacecs]

So I think there are 2 things going on. Firstly, there's the issue that when percentEncodeData doesn't accept an array of objects. So that's where this comes from

{ key: 'filter', value: [ '%5Bobject%20Object%5D' ] }

The other issue is almost the opposite, in that deParam doesn't support decoding arrays from the url parameter string. Which means that when the objects are merged, you end up with all 3 like so

{ key: 'filter', value: [ '%5Bobject%20Object%5D' ] },
{ key: 'filter%255B0%255D%255Battribute%255D', value: 'updated_at' },
{ key: 'filter%255B0%255D%255Bfrom%255D', value: '2017-05-10%2000%3A00%3A00' },

[ddo]

@asharpe-squiz

[robmacecs]

Hi again,
So after more playing around, I figured out I could get this to work by using qs to do some of the encoding/decoding. I don't know whether you would prefer to add qs as a dependency or if you'd rather replicate this functionality in your own module?
Do you have any contribution guidelines?
Thanks

[ddo]

hi,

all the PR should pass the tests, if you can't test on local then you just create a pull request to trigger the semaphoreci.

and also add the new test cases in test/

ty

[paulrutter]

I didn't see this issue earlier, but it looks a lot like the issue i encountered: having multiple duplicate parameters in the querystring doesn't result in a valid signature.
See my pull request: #58
Haven't tested it, but it might fix your issue as well?

[paulrutter]

Ah, looking again i see your issue is about the body parameters (mine was about querystring parameters), so this is probably not fixed by my pull request.

[ddo]

for complicated object data, you should turn it to json string JSON.stringify(data) then send it as a string.