bent0b0x / amazon-mws

MIT License
14 stars 8 forks source link

Query parameters are incorrectly sorted #1

Closed CarmenPopoviciu closed 7 years ago

CarmenPopoviciu commented 8 years ago

Consider the setup below:

mwsClient({
    method: 'POST',
    base: mwsBase,
    endpoint: mwsEndpoint,
    params: {
      'Action': 'ListOrders',
      'CreatedAfter': new Date(params.createdAfter).toISOString(),
      'OrderStatus.Status.1': 'Unshipped',
      'OrderStatus.Status.2': 'PartiallyShipped',
      'MarketplaceId.Id.1': mwsMarketplaceId,
      'SellerId': mwsSellerId,
      'Version': mwsVersion
    });

The API response for the request generated based on those parameters is:

<ErrorResponse xmlns="https://mws.amazonservices.com/Orders/2013-09-01">
  <Error>
    <Type>Sender</Type>
    <Code>SignatureDoesNotMatch</Code>
    <Message>The request signature we calculated does not match the signature you provided. Check your AWS Secret Access Key and signing method. Consult the service documentation for details.</Message>
  </Error>
  <RequestID>...</RequestID>
</ErrorResponse>

After looking into it and comparing the query string generated by the lib with the one returned by the amazon scratchpad, for the exact same request, I noticed that that the order of the query parameters was different. The sorted params array, returned by the library is:

[ [ 'SellerId', '...' ],
  [ 'Action', 'ListOrders' ],
  [ 'AWSAccessKeyId', '...' ],
  [ 'CreatedAfter', '...' ],
  [ 'MarketplaceId.Id.1', '...' ],
  [ 'OrderStatus.Status.1', 'Unshipped' ],
  [ 'OrderStatus.Status.2', 'PartiallyShipped' ],
  [ 'SignatureMethod', 'HmacSHA256' ],
  [ 'SignatureVersion', '2' ],
  [ 'Timestamp', '2016-07-28T12:23:51.683Z' ],
  [ 'Version', '2013-09-01' ] ]

As far as I can tell, the mws docs state that the parameters should be alphabetically ordered, which is obviously not the case for the above. Looking at the code, I see the lib uses this as the sorting function for the params array. AFAIK, sort's compareFunction should return either a positive/negative number or 0, which, unless I'm missing something, the current function in that code, doesn't.

I think this can easily be fixed by just using .sort() with the default behaviour of compareFunction and I would be more than happy to submit a PR for this asap

joanniclaborde commented 7 years ago

+1!

danazkari commented 7 years ago

Seems like this project has been sort of abandoned... do you guys know if this project is still relevant? I don't mind forking and applying the change #2 and using that instead of official repo, if that makes it work properly.

bent0b0x commented 7 years ago

Sorry, did not realize anyone had been using this. Will look into this, generally improving the repo, and merging outstanding PRs.

bent0b0x commented 7 years ago

Fixed by #3