-
Notifications
You must be signed in to change notification settings - Fork 4
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
Limit duration format according to spec #20
Conversation
nexus/api.go
Outdated
@@ -333,3 +335,32 @@ func validateLinkType(value string) error { | |||
} | |||
return nil | |||
} | |||
|
|||
var durationRegexp = regexp.MustCompile("^(\\d+)(ms|s|m|h)$") |
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 think floats should be supported. Spec says "Format of this parameter is number + unit" and I took "number" to mean float as did the Java PR.
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 , the server does generate floats
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.
Also h
is not supported by the spec?
Format of this header value is number + unit, where unit can be
ms
for milliseconds,s
for seconds, andm
for
minutes.
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.
Hmmm... if the server generates floats we'll need to support parsing it for compatibility but I'd stop generating floats on the server ASAP and use ints instead.
Definitely can get rid of h
, thanks for pointing that out, I should've checked, I was looking at @cretz's comment in the Java PR ("s", "m", or "h"
).
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.
Is there a reason we didn't do ISO 8601 durations?
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.
Just felt like overkill and we wanted a format that can be easily typed in a curl command.
34e6136
to
66e899e
Compare
f619c04
to
2439122
Compare
We used to accept anything
time.ParseDuration
would parse which is too broad and violates the spec.Also ensured that we use a custom
formatDuration
implementation to ensure compatibility.