Add util for finding credential helper to use #15707
Conversation
ac79963
to
2f8c78e
Compare
2f8c78e
to
483bbd7
Compare
|
@tjgq PTAL |
| () -> { | ||
| int dot = host.indexOf('.'); | ||
| if (dot < 0) { | ||
| // We reached the last segment, end. | ||
| return Optional.empty(); | ||
| } | ||
| return findWildcardCredentialHelper(host.substring(dot + 1)); |
There was a problem hiding this comment.
I think it would be clearer to encapsulate this logic in a boolean hostMatchesPattern(String pattern, String host) helper.
There was a problem hiding this comment.
Slightly refactored the "get parent domain" logic into helper function.
...java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelperProvider.java
Outdated
Show resolved
Hide resolved
...java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelperProvider.java
Outdated
Show resolved
Hide resolved
| if (pattern.startsWith("*.")) { | ||
| wildcards.put(pattern.substring(2), helper); | ||
| } else { | ||
| hosts.put(pattern, helper); |
There was a problem hiding this comment.
I think we should do more exhaustive validation to reject stuff like foo.*.bar, http://foo.bar, http://foo.bar/path/to/resource, or http://foo.bar:8080.
To be concrete, I think we want the pattern to match the regex (\*|[-a-zA-Z0-9]+)(\.[-a-zA-Z0-9]+)+ (technically domain names are stricter, but this should cover most misuses).
There was a problem hiding this comment.
Done (+ also handling non-ascii DNS names correctly)
...java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelperProvider.java
Outdated
Show resolved
Hide resolved
...java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelperProvider.java
Outdated
Show resolved
Hide resolved
| if (pattern.startsWith("*.")) { | ||
| wildcards.put(pattern.substring(2), helper); | ||
| } else { | ||
| hosts.put(pattern, helper); |
There was a problem hiding this comment.
Done (+ also handling non-ascii DNS names correctly)
| () -> { | ||
| int dot = host.indexOf('.'); | ||
| if (dot < 0) { | ||
| // We reached the last segment, end. | ||
| return Optional.empty(); | ||
| } | ||
| return findWildcardCredentialHelper(host.substring(dot + 1)); |
There was a problem hiding this comment.
Slightly refactored the "get parent domain" logic into helper function.
|
@tjgq ping? |
|
FYI, I fixed a few of the test assertions to use |
|
@bazel-io flag |
|
@bazel-io fork 5.3.0 |
Progress on https://github.com/bazelbuild/proposals/blob/main/designs/2022-06-07-bazel-credential-helpers.md Closes #15707. PiperOrigin-RevId: 458456496 Change-Id: I751a594144c3563096ee9794c41329b49755824e Co-authored-by: Yannic Bonenberger <[email protected]>
Progress on https://github.com/bazelbuild/proposals/blob/main/designs/2022-06-07-bazel-credential-helpers.md