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
Refine the condition uri regex for validation #1597
Refine the condition uri regex for validation #1597
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1597 +/- ##
=======================================
Coverage 98.45% 98.45%
=======================================
Files 58 58
Lines 2850 2850
=======================================
Hits 2806 2806
Misses 44 44 |
f1bc44d
to
9c42b4a
Compare
@@ -154,7 +154,10 @@ definitions: | |||
additionalProperties: true | |||
uri: | |||
type: string | |||
pattern: "^ni:///sha-256;([a-zA-Z0-9_-]{0,86})?(.+)$" | |||
pattern: "^ni:///sha-256;([a-zA-Z0-9_-]{0,86})[?]\ |
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.
I am not sure I understand the repetition on the regex. Is it because you want to have a least one match or because you want to handle the case where you can have a &
?
Have you considered using something like:
"^ni:///sha-256;([a-zA-Z0-9_-]{0,86})[?](fpt=(ed25519|threshold)-sha-256(&)?|cost=[0-9]+(&)?|subtypes=ed25519-sha-256(&)?){1,3}$"
This would eliminate the repetition
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.
Ah ok! Yes, that was for the &
😄
Will make the change!
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.
This will accept URIs ending with &
too, wouldn't it?
bigchaindb/common/transaction.py
Outdated
@@ -13,6 +23,11 @@ | |||
from bigchaindb.common.utils import serialize | |||
|
|||
|
|||
SUPPORTED_CRYPTOCONDITION_TYPES = ('threshold-sha-256', 'ed25519-sha-256') |
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.
I like that we are explicit with what we support and don't support but on the other hand this is only used for testing purposes.
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.
Yes, good point! I can move them both to a test module.
923dcb7
to
3f6bc43
Compare
Yes, I think so, but it's not a big deal. I propose we perform improvements to the regex in other PRs. |
closes #1516