lmccay commented on code in PR #1448:
URL: https://github.com/apache/knox/pull/1448#discussion_r4243643956


##########
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/AudienceResource.java:
##########
@@ -209,4 +229,128 @@ static int indexOfFirst(String value, int fromIndex, char 
a, char b) {
     }
     return Math.min(idxA, idxB);
   }
+
+  /**
+   * Parses a single {@code aud} claim entry as the alternate, DNS-shaped 
destination URL whose
+   * host is {@code <service>.<namespace>.svc.<cluster-domain>}, truncated at 
a label boundary
+   * from the right -- the same shortening an ordinary DNS resolver search 
path allows. Returns
+   * {@link Optional#empty()} for any entry that is not a well-formed URL of 
that shape --
+   * including a {@code null} or blank entry, a non-{@code https} scheme, a 
missing or
+   * userinfo-carrying authority, a host with a trailing {@code "."}, an empty 
host label, a host
+   * of three or more labels whose third label is not literally {@code "svc"}, 
a host whose labels
+   * past {@code "svc"} are not a label-boundary prefix of {@code 
clusterDomain} taken from its
+   * left, or a resource path that is not already in RFC 3986 
remove_dot_segments normal form.
+   *
+   * <p>The result's {@code namespace} is {@code null} when the host is a 
single label (service
+   * name only); a namespace-less host has no destination-namespace 
information of its own, and it
+   * is entirely up to the caller matching this result to decide what, if 
anything, it is compared
+   * against. {@code host} and {@code effectivePort} are not meaningful for a 
DNS-form result and
+   * are always set to {@code null} and {@code -1} respectively -- the 
cluster-domain host and any
+   * port in the entry are consumed only to parse the authority and are not 
retained; see the next
+   * paragraph for why the port specifically is discarded.
+   *
+   * <p>The port, when present, is parsed only so that it does not break 
parsing of the rest of the
+   * authority. It is then discarded rather than retained in the result: a 
mesh or sidecar in front
+   * of the destination may rewrite the port in flight, so pinning it here 
would risk rejecting a
+   * correctly-routed request.
+   *
+   * <p>The resource path here is never prefix-searched the way {@link 
#parseCustomForm(String,
+   * String)}'s is: a DNS-form entry has nowhere else for namespace and 
service-name to live, so
+   * the whole path -- after tidying -- is the resource path.
+   *
+   * <p>A {@code ?query} or {@code #fragment} on the entry is discarded before 
the path is parsed,
+   * exactly as in {@link #parseCustomForm(String, String)}.
+   *
+   * @param audEntry the raw {@code aud} claim entry to parse
+   * @param clusterDomain the cluster domain a host of three or more labels 
must truncate to a
+   *     label-boundary prefix of, past the required {@code "svc"} label, as 
described above
+   * @return the parsed result, or {@link Optional#empty()} if {@code 
audEntry} is not a
+   *     well-formed DNS-form URL as described above
+   */
+  static Optional<AudienceResource> parseDnsForm(String audEntry, String 
clusterDomain) {
+    if (audEntry == null) {
+      return Optional.empty();
+    }
+    final String entry = audEntry.trim();
+    if (entry.isEmpty()) {
+      return Optional.empty();
+    }
+
+    final int schemeSep = entry.indexOf("://");
+    if (schemeSep <= 0 || !"https".equalsIgnoreCase(entry.substring(0, 
schemeSep))) {
+      return Optional.empty();
+    }
+
+    final int pathStart = entry.indexOf('/', schemeSep + 3);
+    final String authorityPart;
+    String rawPath;
+    if (pathStart < 0) {
+      authorityPart = entry;
+      rawPath = "/";
+    } else {
+      authorityPart = entry.substring(0, pathStart);
+      rawPath = entry.substring(pathStart);
+    }
+    final int queryOrFragment = indexOfFirst(rawPath, 0, '?', '#');
+    if (queryOrFragment >= 0) {
+      rawPath = rawPath.substring(0, queryOrFragment);
+    }

Review Comment:
   This seems like a valid critique.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to