Apply httpparams more universally

- add config to forbid users from using transport directly
- make tuf/oidc clients use requestfactory when possible

Signed-off-by: Appu Goundan <[email protected]>

Author: loosebazooka

config/forbiddenApis.txt

@@ -1 +1,2 @@
 com.google.protobuf.util.JsonFormat#parser() @ Use dev.sigstore.json.ProtoJson#parser() instead
+dev.sigstore.http.HttpClients#newHttpTransport(dev.sigstore.http.HttpParams) @ Use dev.sigstore.http.HttpClients#newRequestFactory(...) instead

sigstore-java/src/main/java/dev/sigstore/http/HttpClients.java

@@ -20,16 +20,20 @@
 import com.google.api.client.http.HttpTransport;
 import com.google.api.client.http.apache.v2.ApacheHttpTransport;
 import com.google.api.client.util.ExponentialBackOff;
+import com.google.api.client.util.ObjectParser;
+import dev.sigstore.forbidden.SuppressForbidden;
 import java.io.IOException;
+import javax.annotation.Nullable;
 import org.apache.http.conn.ssl.NoopHostnameVerifier;
 import org.apache.http.impl.client.HttpClientBuilder;
 
 /** HttpClients generates Google Http Client objects from configuration. */
 public class HttpClients {
 
   /**
-   * Build a transport, you probably want to use {@link #newRequestFactory} to instantiate GET and
-   * POST requests.
+   * Build a transport, you probably want to use {@link #newRequestFactory(HttpParams)} to
+   * instantiate GET and POST requests or use {@link #newRequestFactory(HttpParams, ObjectParser) if
+   * you need to also configure the response parser}.
    */
   public static HttpTransport newHttpTransport(HttpParams httpParams) {
     HttpClientBuilder hcb =
@@ -41,7 +45,15 @@ public static HttpTransport newHttpTransport(HttpParams httpParams) {
   }
 
   /** Create a new get requests with the httpParams applied and retries. */
+  @SuppressForbidden(reason = "HttpClients#newHttpTransport(HttpParams)")
   public static HttpRequestFactory newRequestFactory(HttpParams httpParams) throws IOException {
+    return newRequestFactory(httpParams, null);
+  }
+
+  /** Create a new get requests with the httpParams applied, retries and a response parser. */
+  @SuppressForbidden(reason = "HttpClients#newHttpTransport(HttpParams)")
+  public static HttpRequestFactory newRequestFactory(
+      HttpParams httpParams, @Nullable ObjectParser responseParser) throws IOException {
     return HttpClients.newHttpTransport(httpParams)
         .createRequestFactory(
             request -> {
@@ -52,6 +64,9 @@ public static HttpRequestFactory newRequestFactory(HttpParams httpParams) throws
                   UnsuccessfulResponseHandler.newUnsuccessfulResponseHandler());
               request.setIOExceptionHandler(
                   new HttpBackOffIOExceptionHandler(new ExponentialBackOff()));
+              if (responseParser != null) {
+                request.setParser(responseParser);
+              }
             });
   }
 }

sigstore-java/src/main/java/dev/sigstore/oidc/client/WebOidcClient.java

@@ -24,7 +24,6 @@
 import com.google.api.client.extensions.jetty.auth.oauth2.LocalServerReceiver;
 import com.google.api.client.http.GenericUrl;
 import com.google.api.client.http.HttpRequest;
-import com.google.api.client.http.HttpRequestFactory;
 import com.google.api.client.http.HttpTransport;
 import com.google.api.client.json.GenericJson;
 import com.google.api.client.json.JsonFactory;
@@ -33,6 +32,7 @@
 import com.google.api.client.util.Preconditions;
 import com.google.api.client.util.store.DataStoreFactory;
 import com.google.api.client.util.store.MemoryDataStoreFactory;
+import dev.sigstore.forbidden.SuppressForbidden;
 import dev.sigstore.http.HttpClients;
 import dev.sigstore.http.HttpParams;
 import dev.sigstore.trustroot.Service;
@@ -140,13 +140,14 @@ public boolean isEnabled(Map<String, String> env) {
    * @throws OidcException if an error occurs doing the authorization flow
    */
   @Override
+  @SuppressForbidden(reason = "HttpClients#newHttpTransport(HttpParams)")
   public OidcToken getIDToken(Map<String, String> env) throws OidcException {
     JsonFactory jsonFactory = new GsonFactory();
     HttpTransport httpTransport = HttpClients.newHttpTransport(httpParams);
     DataStoreFactory memStoreFactory = new MemoryDataStoreFactory();
     OIDCEndpoints endpoints;
     try {
-      endpoints = parseDiscoveryDocument(jsonFactory, httpTransport);
+      endpoints = parseDiscoveryDocument(jsonFactory);
     } catch (IOException e) {
       // TODO: maybe a more descriptive exception message
       throw new OidcException(
@@ -209,15 +210,12 @@ public OidcToken getIDToken(Map<String, String> env) throws OidcException {
         .build();
   }
 
-  // Parses a oidc discovery document to discover other endpoints. This method does not
+  // Parses an oidc discovery document to discover other endpoints. This method does not
   // parse all the values, only the endpoints we care about.
-  OIDCEndpoints parseDiscoveryDocument(JsonFactory jsonFactory, HttpTransport httpTransport)
-      throws IOException {
-    HttpRequestFactory requestFactory =
-        httpTransport.createRequestFactory(
-            request -> {
-              request.setParser(jsonFactory.createJsonObjectParser());
-            });
+  OIDCEndpoints parseDiscoveryDocument(JsonFactory jsonFactory) throws IOException {
+    var requestFactory =
+        HttpClients.newRequestFactory(
+            HttpParams.builder().build(), jsonFactory.createJsonObjectParser());
     GenericUrl wellKnownConfig = new GenericUrl(issuer);
     wellKnownConfig.appendRawPath(WELL_KNOWN_CONFIG);
     HttpRequest request = requestFactory.buildGetRequest(wellKnownConfig);

sigstore-java/src/main/java/dev/sigstore/tuf/HttpFetcher.java

@@ -16,27 +16,33 @@
 package dev.sigstore.tuf;
 
 import com.google.api.client.http.GenericUrl;
+import com.google.api.client.http.HttpRequestFactory;
 import com.google.api.client.json.gson.GsonFactory;
 import dev.sigstore.http.HttpClients;
 import dev.sigstore.http.HttpParams;
 import java.io.IOException;
-import java.net.MalformedURLException;
 import java.net.URL;
 import java.util.Locale;
 
 public class HttpFetcher implements Fetcher {
 
   private final URL mirror;
+  private final HttpRequestFactory requestFactory;
 
-  private HttpFetcher(URL mirror) {
+  private HttpFetcher(URL mirror, HttpRequestFactory requestFactory) {
     this.mirror = mirror;
+    this.requestFactory = requestFactory;
   }
 
-  public static HttpFetcher newFetcher(URL mirror) throws MalformedURLException {
+  public static HttpFetcher newFetcher(URL mirror) throws IOException {
+    var requestFactory =
+        HttpClients.newRequestFactory(
+            HttpParams.builder().build(),
+            GsonFactory.getDefaultInstance().createJsonObjectParser());
     if (mirror.toString().endsWith("/")) {
-      return new HttpFetcher(mirror);
+      return new HttpFetcher(mirror, requestFactory);
     }
-    return new HttpFetcher(new URL(mirror.toExternalForm() + "/"));
+    return new HttpFetcher(new URL(mirror.toExternalForm() + "/"), requestFactory);
   }
 
   @Override
@@ -48,12 +54,7 @@ public String getSource() {
   public byte[] fetchResource(String filename, int maxLength)
       throws IOException, FileExceedsMaxLengthException {
     GenericUrl fileUrl = new GenericUrl(mirror + filename);
-    var req =
-        HttpClients.newHttpTransport(HttpParams.builder().build())
-            .createRequestFactory(
-                request ->
-                    request.setParser(GsonFactory.getDefaultInstance().createJsonObjectParser()))
-            .buildGetRequest(fileUrl);
+    var req = requestFactory.buildGetRequest(fileUrl);
     req.getHeaders().setAccept("application/json; api-version=2.0");
     req.getHeaders().setContentType("application/json");
     req.setThrowExceptionOnExecuteError(false);