Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issues with date starting on month 10 #30

Closed
bitadvisor opened this issue Oct 1, 2012 · 2 comments
Closed

Issues with date starting on month 10 #30

bitadvisor opened this issue Oct 1, 2012 · 2 comments

Comments

@bitadvisor
Copy link

Starting from 2012-10-01 we get the following error:

error: dynode.batchGetItem returned error: AmazonError - BatchGetItem 400 IncompleteSignatureException: Date must be in ISO-8601 'basic format'. Got '202201T000748Z'. See http://en.wikipedia.org/wiki/ISO_8601

Please note that the date is 202201 instead of 20121001. This is a variable type isue - number vs string - in aws-signer.js lines 65-70 and line 90.

To resolve, change line 90 to:

return n < 10 ? '0'+n : String(n);
@ryanfitz
Copy link
Member

ryanfitz commented Oct 1, 2012

This is a major bug and here are more details.

i have been using dynode for a few months now to build a few node.js
systems that access dynamoDB. just a few hours ago, i noticed that
all of our systems using dynode started failing to connect to
dynamoDB. i went through your code and isolated this to a date string
conversion bug in lib/dynode/aws-signer.js. this bug only started
kicking in once the month turned double-digit, which by UTC time just
happened a few hours ago.

using this function as an example to explain the bug:

Signer._requestDate = function(date) {
return date.getUTCFullYear() +
pad(date.getUTCMonth()+1) +
pad(date.getUTCDate())+'T' +
pad(date.getUTCHours()) +
pad(date.getUTCMinutes()) +
pad(date.getUTCSeconds())+'Z';
}

each of these date.get*() functions return components that need to be
concatenated into the amazon date string, i.e. "2012101T023723Z". the
pad function called ahead checks for single-digit numbers and expands
them to have a '0' in front, as following:

function pad(n) {
return n<10 ? '0'+ n : n;
}

once the date becomes a double-digit number, it does not get converted
to a string as part of the padding, and as a result the "+" operator
adding month to year does a numerical addition, producing date strings
like "202201T023723Z". this is causing amazon to reject every dynode
request with errors like this:

{
"type": "InvalidSignatureException",
"serviceName": "com.amazon.coral.service",
"message": "Date in Credential scope does not match YYYYMMDD from
ISO-8601 version of date from HTTP: '202201' != '20121001', from
'20121001T024252Z'.",
"statusCode": 400,
"action": "DescribeTable",
"retry": false
}

if i understand the impact of this bug correctly, this will
immediately break all dynamoDB connections for anyone using dynode.

fortunately a fix can be made fairly simply by adding an explicitly
toString() conversion to the year in both parts of
lib/dynode/aws-signer.js that perform this operation:

Signer._requestDate = function(date) {
return (date.getUTCFullYear()).toString() +
pad(date.getUTCMonth()+1) +
pad(date.getUTCDate())+'T' +
pad(date.getUTCHours()) +
pad(date.getUTCMinutes()) +
pad(date.getUTCSeconds())+'Z';
};

function today(date) {
return (date.getUTCFullYear()).toString() +
pad(date.getUTCMonth()+1) + pad(date.getUTCDate());
}

@jacklevy
Copy link

jacklevy commented Oct 1, 2012

actually on further review i think the solution proposed by psprindys is actually better. it ensures that pad() always returns a string, which is really a good post-condition for it. could prevent future similar bugs from further use of pad().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants