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

fix(packet): add precheck for timestamps in Prepares #707

Merged
merged 5 commits into from
Apr 16, 2021

Conversation

whalelephant
Copy link
Contributor

@whalelephant whalelephant commented Apr 15, 2021

Cc: #705
Check if bytes are numerical values before handing to chrono

@whalelephant whalelephant marked this pull request as ready for review April 15, 2021 08:31
Copy link
Collaborator

@koivunej koivunej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good!

Could do a git rebase --signoff onto the repos master branch?

@whalelephant whalelephant force-pushed the whalelephant branch 2 times, most recently from 2a89cc4 to 5e5c8c1 Compare April 16, 2021 11:57
…rono

Signed-off-by: bwty <whalelephant@users.noreply.github.com>

test: datetime ensure error with invalid characters

Signed-off-by: bwty <whalelephant@users.noreply.github.com>

doc: Prepare package datetime spec

Signed-off-by: bwty <whalelephant@users.noreply.github.com>

Signed-off-by: bwty <whalelephant@users.noreply.github.com>
Signed-off-by: bwty <whalelephant@users.noreply.github.com>
Signed-off-by: bwty <whalelephant@users.noreply.github.com>
Signed-off-by: bwty <whalelephant@users.noreply.github.com>
Signed-off-by: bwty <whalelephant@users.noreply.github.com>
@koivunej koivunej changed the title fix(packet): add regex check for time in packet fix(packet): add precheck for timestamps in Prepares Apr 16, 2021
@koivunej
Copy link
Collaborator

koivunej commented Apr 16, 2021

This is a correctness update which will deny some patterns which were previously accepted as valid timestamps, but did not adhere to RFC and the asn1 specification. The previously accepted timestamps can be seen with the test case. Merging slightly ahead of the 24h window.

@koivunej koivunej merged commit 527e25e into interledger:master Apr 16, 2021
Mirko-von-Leipzig pushed a commit to Mirko-von-Leipzig/interledger-rs that referenced this pull request Aug 3, 2021
this follows from finding a case where the parsed timestamp doesn't
match the output timestamp, even with the fix made in interledger#707. the strict
feature is included in the "rountrip-only".

Signed-off-by: Joonas Koivunen <joonas.koivunen@gmail.com>
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

Successfully merging this pull request may close these issues.

2 participants