fixes #1107 #1108
fixes #1107 #1108
Conversation
|
Can one of the admins verify this patch? |
cmd/notary/tuf.go
Outdated
| @@ -889,7 +889,13 @@ func tokenAuth(trustServerURL string, baseTransport *http.Transport, gun data.GU | |||
| if endpoint.Scheme == "" { | |||
| return nil, fmt.Errorf("Trust server url has to be in the form of http(s)://URL:PORT. Got: %s", trustServerURL) | |||
| } | |||
| subPath, err := url.Parse("v2/") | |||
| var p string | |||
| if strings.HasSuffix(endpoint.Path, "/") { | |||
There was a problem hiding this comment.
I think you can just use filepath.Join here, which would take care of de-duping the / for you (and even de-duping extra / as well: https://play.golang.org/p/ALPqJzKDCq)
There was a problem hiding this comment.
Non-blocking: If you wouldn't mind, could this URL validation be moved to a utility function in storage/httpstore.go so that it could be shared between this and the HTTPStore? It'd be useful for library users of the notary client as well.
It would also make it easier to test, since you can just test that function alone (because the second thing I was going to ask is a test for this particular validation withsubpaths :))
There was a problem hiding this comment.
@cyli should just be path.Join as filepath would do the wrong thing on Windows systems and these are HTTP URLs.
Could alternatively use the url package which will absolutely ensure it's a correct URL.
There was a problem hiding this comment.
You have to be precise with the / but it works. https://play.golang.org/p/eVHFrPre4i
There was a problem hiding this comment.
@endophage Ah good point, thanks.
There was a problem hiding this comment.
@endophage
Are you saying user have to call
notary -s https://docker.com/notary/??
Wouldn't it be better if the code can handle both cases, with or without the trailing /
There was a problem hiding this comment.
@cyli
Thanks for reviewing I'll check httpstore.go when I have time, probably early next week.
There was a problem hiding this comment.
Well... I was thinking we could detect the trailing slash and add if necessary. I don't feel strongly though, what you have currently is fine.
|
jenkins, test this please |
|
The circleCI failures appear legitimate. It appears |
|
@endophage Thanks for the comment. |
Signed-off-by: Tan Jiang <[email protected]>
|
@reasonerjt no worries, but it does make me think we should detect the presence of a trailing |
|
jenkins, test this please |
|
LGTM pending Jenkins but fully expect it to pass. |
When doing auth, add the path of "tuf URL" before "/v2" for ping client
For more details please refer to #1107
Signed-off-by: Tan Jiang [email protected]