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
HPCC-21267 Possible issue in Dali regarding timestamp comparison #12049
HPCC-21267 Possible issue in Dali regarding timestamp comparison #12049
Conversation
When Dali checks the request timestamp to see if it is expired or from the future, it should ignore nanoseconds in order to eliminate occasional unexpected failures. Also, in the case of a failure, add logging of the given request time stamp and the actual Dali time stamp, both in UTC, which should help debug failures. Signed-off-by: Russ Whitehead <william.whitehead@lexisnexisrisk.com>
https://track.hpccsystems.com/browse/HPCC-21267 |
@rpastrana Please review |
Automated Smoketest: ✅ Unit tests result:
Regression test result:
HPCC Stop: OK
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RussWhitehead looks reasonable, just a couple of questions.
@@ -164,19 +164,23 @@ class CDaliLdapConnection: implements IDaliLdapConnection, public CInterface | |||
|
|||
CDateTime now; | |||
now.setNow(); | |||
if (now.compare(reqUTCTimestamp) < 0)//timestamp from the future? | |||
if (now.compare(reqUTCTimestamp, false) < 0)//timestamp from the future? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we ensure the req timestamp is in UTC?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is set by the caller (runtime engine), in dasess.cpp method createDaliSignature, which does this
StringBuffer timeStr;
now.setNow();
now.getString(timeStr, false);//get UTC timestamp
@richardkchapman Please merge |
@rpastrana Is this approved now? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RussWhitehead approved
@richardkchapman Please merge |
@richardkchapman The Vault team is asking for this. If there is another 7.0.6 release coming out let me know and I can rebase |
When Dali checks the request timestamp to see if it is expired or from the
future, it should ignore nanoseconds in order to eliminate occasional
unexpected failures. Also, in the case of a failure, add logging of the given
request time stamp and the actual Dali time stamp, both in UTC, which should
help debug failures.
Signed-off-by: Russ Whitehead william.whitehead@lexisnexisrisk.com
Type of change:
Checklist:
Testing: