Instead of an InputStream that doesn't know about its encoding, use a…

… String Closes keycloak#20916 Signed-off-by: Alexander Schwartz <[email protected]>
ebremer · Mar 7, 2024 · 5959593 · 5959593
1 parent b459630
commit 5959593
Show file tree

Hide file tree

Showing 18 changed files with 122 additions and 100 deletions.
diff --git a/saml-core/src/main/java/org/keycloak/saml/common/util/StaxParserUtil.java b/saml-core/src/main/java/org/keycloak/saml/common/util/StaxParserUtil.java
@@ -52,6 +52,7 @@
 import javax.xml.validation.SchemaFactory;
 import javax.xml.validation.Validator;
 import java.io.InputStream;
+import java.io.StringReader;
 import java.net.URI;
 import java.util.ArrayList;
 import java.util.List;
@@ -559,6 +560,16 @@ public static XMLEventReader getXMLEventReader(InputStream is) {
         return xmlEventReader;
     }
 
+    public static XMLEventReader getXMLEventReader(String xml) {
+        XMLInputFactory xmlInputFactory;
+        xmlInputFactory = XML_INPUT_FACTORY.get();
+        try {
+            return xmlInputFactory.createXMLEventReader(new StringReader(xml));
+        } catch (Exception ex) {
+            throw new RuntimeException(ex);
+        }
+    }
+
     private static AtomicBoolean XML_EVENT_READER_ON_SOURCE_SUPPORTED = new AtomicBoolean(true);
 
     /**

diff --git a/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/util/SAMLMetadataUtil.java b/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/util/SAMLMetadataUtil.java
@@ -16,7 +16,6 @@
  */
 package org.keycloak.saml.processing.core.saml.v2.util;
 
-import java.io.InputStream;
 import java.security.cert.X509Certificate;
 import java.util.List;
 import java.util.function.Function;
@@ -31,6 +30,7 @@
 import org.keycloak.saml.common.exceptions.ConfigurationException;
 import org.keycloak.saml.common.exceptions.ParsingException;
 import org.keycloak.saml.common.exceptions.ProcessingException;
+import org.keycloak.saml.common.util.StaxParserUtil;
 import org.keycloak.saml.processing.core.parsers.saml.SAMLParser;
 import org.keycloak.saml.processing.core.util.XMLSignatureUtil;
 import org.w3c.dom.Element;
@@ -106,8 +106,8 @@ public static X509Certificate getCertificate(KeyTypes use, SSODescriptorType sso
         return null;
     }
 
-    public static EntityDescriptorType parseEntityDescriptorType(InputStream inputStream) throws ParsingException {
-        Object parsedObject = SAMLParser.getInstance().parse(inputStream);
+    public static EntityDescriptorType parseEntityDescriptorType(String descriptor) throws ParsingException {
+        Object parsedObject = SAMLParser.getInstance().parse(StaxParserUtil.getXMLEventReader(descriptor));
         EntityDescriptorType entityType;
 
         if (EntitiesDescriptorType.class.isInstance(parsedObject)) {

diff --git a/...i-private/src/main/java/org/keycloak/broker/provider/AbstractIdentityProviderFactory.java b/...i-private/src/main/java/org/keycloak/broker/provider/AbstractIdentityProviderFactory.java
@@ -20,7 +20,6 @@
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.KeycloakSessionFactory;
 
-import java.io.InputStream;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -50,7 +49,7 @@ public T create(KeycloakSession session) {
     }
 
     @Override
-    public Map<String, String> parseConfig(KeycloakSession session, InputStream inputStream) {
+    public Map<String, String> parseConfig(KeycloakSession session, String config) {
         return new HashMap<>();
     }
 }
diff --git a/server-spi-private/src/main/java/org/keycloak/broker/provider/IdentityProviderFactory.java b/server-spi-private/src/main/java/org/keycloak/broker/provider/IdentityProviderFactory.java
@@ -54,10 +54,10 @@ public interface IdentityProviderFactory<T extends IdentityProvider> extends Pro
      * <code>inputStream</code>.</p>
      *
      * @param session
-     * @param inputStream The input stream from where configuration will be loaded from..
+     * @param config The configuration for the provider
      * @return
      */
-    Map<String, String> parseConfig(KeycloakSession session, InputStream inputStream);
+    Map<String, String> parseConfig(KeycloakSession session, String config);
 
     /**
      * <p>Creates a provider specific {@link IdentityProviderModel} instance.

diff --git a/server-spi-private/src/main/java/org/keycloak/connections/httpclient/HttpClientProvider.java b/server-spi-private/src/main/java/org/keycloak/connections/httpclient/HttpClientProvider.java
@@ -21,6 +21,7 @@
 
 import java.io.IOException;
 import java.io.InputStream;
+
 import org.apache.http.impl.client.CloseableHttpClient;
 
 /**
@@ -50,11 +51,43 @@ public interface HttpClientProvider extends Provider {
     public int postText(String uri, String text) throws IOException;
 
     /**
-     * Helper method
+     * Helper method to retrieve the contents of a URL as a String.
+     * Decoding response with the correct character set is performed according to the headers returned in the server's response.
+     * To retrieve binary data, use {@link #getInputStream(String)}
      *
-     * @param uri
-     * @return response stream, you must close this stream or leaks will happen
+     * @param uri URI with data to receive.
+     * @return Body of the response as a String.
      * @throws IOException On network errors, no content being returned or a non-2xx HTTP status code
      */
-    public InputStream get(String uri) throws IOException;
+    String getString(String uri) throws IOException;
+
+    /**
+     * Helper method to retrieve the contents of a URL as an InputStream.
+     * Use this to retrieve binary data where no additional HTTP headers need to be considered.
+     * The caller is required to close the returned InputStream to prevent a resource leak.
+     * <p>
+     * To retrieve strings that depend on their encoding, use {@link #getString(String)}
+     *
+     * @param uri URI with data to receive.
+     * @return Body of the response as an InputStream. The caller is required to close the returned InputStream to prevent a resource leak.
+     * @throws IOException On network errors, no content being returned or a non-2xx HTTP status code.
+     */
+    InputStream getInputStream(String uri) throws IOException;
+
+    /**
+     * Helper method.
+     * The caller is required to close the returned InputStream to prevent a resource leak.
+
+     * @deprecated For String content, use  {@link #getString(String)}, for binary data use {@link #getInputStream(String)}.
+     * To be removed in Keycloak 27.
+     *
+     * @param uri URI with data to receive.
+     * @return Body of the response as an InputStream. The caller is required to close the returned InputStream to prevent a resource leak.
+     * @throws IOException On network errors, no content being returned or a non-2xx HTTP status code.
+     */
+    @Deprecated
+    default InputStream get(String uri) throws IOException {
+        return getInputStream(uri);
+    }
+
 }
diff --git a/services/src/main/java/org/keycloak/broker/oidc/KeycloakOIDCIdentityProviderFactory.java b/services/src/main/java/org/keycloak/broker/oidc/KeycloakOIDCIdentityProviderFactory.java
@@ -20,7 +20,6 @@
 import org.keycloak.models.IdentityProviderModel;
 import org.keycloak.models.KeycloakSession;
 
-import java.io.InputStream;
 import java.util.Map;
 
 /**
@@ -46,8 +45,8 @@ public String getId() {
     }
 
     @Override
-    public Map<String, String> parseConfig(KeycloakSession session, InputStream inputStream) {
-        return OIDCIdentityProviderFactory.parseOIDCConfig(session, inputStream);
+    public Map<String, String> parseConfig(KeycloakSession session, String config) {
+        return OIDCIdentityProviderFactory.parseOIDCConfig(session, config);
     }
 
     @Override

diff --git a/services/src/main/java/org/keycloak/broker/oidc/OIDCIdentityProviderFactory.java b/services/src/main/java/org/keycloak/broker/oidc/OIDCIdentityProviderFactory.java
@@ -23,7 +23,6 @@
 import org.keycloak.util.JsonSerialization;
 
 import java.io.IOException;
-import java.io.InputStream;
 import java.util.Map;
 
 /**
@@ -54,14 +53,14 @@ public String getId() {
     }
 
     @Override
-    public Map<String, String> parseConfig(KeycloakSession session, InputStream inputStream) {
-        return parseOIDCConfig(session, inputStream);
+    public Map<String, String> parseConfig(KeycloakSession session, String config) {
+        return parseOIDCConfig(session, config);
     }
 
-    protected static Map<String, String> parseOIDCConfig(KeycloakSession session, InputStream inputStream) {
+    protected static Map<String, String> parseOIDCConfig(KeycloakSession session, String configString) {
         OIDCConfigurationRepresentation rep;
         try {
-            rep = JsonSerialization.readValue(inputStream, OIDCConfigurationRepresentation.class);
+            rep = JsonSerialization.readValue(configString, OIDCConfigurationRepresentation.class);
         } catch (IOException e) {
             throw new RuntimeException("failed to load openid connect metadata", e);
         }

diff --git a/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderFactory.java b/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderFactory.java
@@ -16,7 +16,6 @@
  */
 package org.keycloak.broker.saml;
 
-import java.io.InputStream;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
@@ -69,9 +68,9 @@ public SAMLIdentityProviderConfig createConfig() {
     }
 
     @Override
-    public Map<String, String> parseConfig(KeycloakSession session, InputStream inputStream) {
+    public Map<String, String> parseConfig(KeycloakSession session, String config) {
         try {
-            EntityDescriptorType entityType = SAMLMetadataUtil.parseEntityDescriptorType(inputStream);
+            EntityDescriptorType entityType = SAMLMetadataUtil.parseEntityDescriptorType(config);
             IDPSSODescriptorType idpDescriptor = SAMLMetadataUtil.locateIDPSSODescriptorType(entityType);
 
             if (idpDescriptor != null) {

diff --git a/services/src/main/java/org/keycloak/connections/httpclient/DefaultHttpClientFactory.java b/services/src/main/java/org/keycloak/connections/httpclient/DefaultHttpClientFactory.java
@@ -23,6 +23,8 @@
 import org.apache.http.client.methods.HttpGet;
 import org.apache.http.client.methods.HttpPost;
 import org.apache.http.entity.ContentType;
+import org.apache.http.impl.client.AbstractResponseHandler;
+import org.apache.http.impl.client.BasicResponseHandler;
 import org.apache.http.impl.client.CloseableHttpClient;
 import org.jboss.logging.Logger;
 import org.keycloak.Config;
@@ -75,6 +77,21 @@ public class DefaultHttpClientFactory implements HttpClientFactory {
     private volatile CloseableHttpClient httpClient;
     private Config.Scope config;
 
+    private final BasicResponseHandler stringResponseHandler = new BasicResponseHandler();
+
+    private final InputStreamResponseHandler inputStreamResponseHandler = new InputStreamResponseHandler();
+
+    private static class InputStreamResponseHandler extends AbstractResponseHandler<InputStream> {
+
+        public InputStream handleEntity(HttpEntity entity) throws IOException {
+            return entity.getContent();
+        }
+
+        public InputStream handleResponse(HttpResponse response) throws IOException {
+            return super.handleResponse(response);
+        }
+    }
+
     @Override
     public HttpClientProvider create(KeycloakSession session) {
         lazyInit(session);
@@ -107,20 +124,25 @@ public int postText(String uri, String text) throws IOException {
             }
 
             @Override
-            public InputStream get(String uri) throws IOException {
+            public String getString(String uri) throws IOException {
                 HttpGet request = new HttpGet(uri);
                 HttpResponse response = httpClient.execute(request);
-                int statusCode = response.getStatusLine().getStatusCode();
-                HttpEntity entity = response.getEntity();
-                if (statusCode < 200 || statusCode >= 300) {
-                    EntityUtils.consumeQuietly(entity);
-                    throw new IOException("Unexpected HTTP status code " + response.getStatusLine().getStatusCode() + " when expecting 2xx");
-                }
-                if (entity == null) {
+                String body = stringResponseHandler.handleResponse(response);
+                if (body == null) {
                     throw new IOException("No content returned from HTTP call");
                 }
-                return entity.getContent();
+                return body;
+            }
 
+            @Override
+            public InputStream getInputStream(String uri) throws IOException {
+                HttpGet request = new HttpGet(uri);
+                HttpResponse response = httpClient.execute(request);
+                InputStream body = inputStreamResponseHandler.handleResponse(response);
+                if (body == null) {
+                    throw new IOException("No content returned from HTTP call");
+                }
+                return body;
             }
         };
     }

diff --git a/...keycloak/protocol/oidc/endpoints/request/AuthorizationEndpointRequestParserProcessor.java b/...keycloak/protocol/oidc/endpoints/request/AuthorizationEndpointRequestParserProcessor.java
@@ -20,7 +20,6 @@
 import org.jboss.logging.Logger;
 import org.keycloak.OAuth2Constants;
 import org.keycloak.common.Profile;
-import org.keycloak.common.util.StreamUtil;
 import org.keycloak.connections.httpclient.HttpClientProvider;
 import org.keycloak.events.Errors;
 import org.keycloak.events.EventBuilder;
@@ -39,7 +38,6 @@
 
 import jakarta.ws.rs.core.MultivaluedMap;
 import jakarta.ws.rs.core.Response;
-import java.io.InputStream;
 import java.util.HashSet;
 import java.util.List;
 
@@ -98,10 +96,8 @@ public static AuthorizationEndpointRequest parseRequest(EventBuilder event, Keyc
                     if (requestUri == null) {
                         throw new RuntimeException("Specified 'request_uri' not allowed for this client.");
                     }
-                    try (InputStream is = session.getProvider(HttpClientProvider.class).get(requestUri)) {
-                        String retrievedRequest = StreamUtil.readString(is);
-                        new AuthzEndpointRequestObjectParser(session, retrievedRequest, client).parseRequest(request);
-                    }
+                    String retrievedRequest = session.getProvider(HttpClientProvider.class).getString(requestUri);
+                    new AuthzEndpointRequestObjectParser(session, retrievedRequest, client).parseRequest(request);
                 }
             }
 

diff --git a/...rants/ciba/endpoints/request/BackchannelAuthenticationEndpointRequestParserProcessor.java b/...rants/ciba/endpoints/request/BackchannelAuthenticationEndpointRequestParserProcessor.java
@@ -19,7 +19,6 @@
 package org.keycloak.protocol.oidc.grants.ciba.endpoints.request;
 
 import org.keycloak.OAuthErrorException;
-import org.keycloak.common.util.StreamUtil;
 import org.keycloak.connections.httpclient.HttpClientProvider;
 import org.keycloak.events.EventBuilder;
 import org.keycloak.models.CibaConfig;
@@ -32,7 +31,6 @@
 
 import jakarta.ws.rs.core.MultivaluedMap;
 import jakarta.ws.rs.core.Response;
-import java.io.InputStream;
 import java.util.HashSet;
 import java.util.List;
 
@@ -70,10 +68,8 @@ public static BackchannelAuthenticationEndpointRequest parseRequest(EventBuilder
                     throw new RuntimeException("Specified 'request_uri' not allowed for this client.");
                 }
 
-                try (InputStream is = session.getProvider(HttpClientProvider.class).get(requestUri)) {
-                    String retrievedRequest = StreamUtil.readString(is);
-                    new BackchannelAuthenticationEndpointSignedRequestParser(session, retrievedRequest, client, config).parseRequest(request);
-                }
+                String retrievedRequest = session.getProvider(HttpClientProvider.class).getString(requestUri);
+                new BackchannelAuthenticationEndpointSignedRequestParser(session, retrievedRequest, client, config).parseRequest(request);
             }
 
             return request;

diff --git a/...a/org/keycloak/protocol/oidc/par/endpoints/request/ParEndpointRequestParserProcessor.java b/...a/org/keycloak/protocol/oidc/par/endpoints/request/ParEndpointRequestParserProcessor.java
@@ -16,15 +16,13 @@
  */
 package org.keycloak.protocol.oidc.par.endpoints.request;
 
-import java.io.InputStream;
 import java.util.HashSet;
 import java.util.List;
 
 import jakarta.ws.rs.core.MultivaluedMap;
 import jakarta.ws.rs.core.Response;
 
 import org.keycloak.common.Profile;
-import org.keycloak.common.util.StreamUtil;
 import org.keycloak.connections.httpclient.HttpClientProvider;
 import org.keycloak.events.Errors;
 import org.keycloak.events.EventBuilder;
@@ -89,10 +87,8 @@ public static AuthorizationEndpointRequest parseRequest(EventBuilder event, Keyc
                 if (requestUri == null) {
                     throw new RuntimeException("Specified 'request_uri' not allowed for this client.");
                 }
-                try (InputStream is = session.getProvider(HttpClientProvider.class).get(requestUri)) {
-                    String retrievedRequest = StreamUtil.readString(is);
-                    new ParEndpointRequestObjectParser(session, retrievedRequest, client).parseRequest(request);
-                }
+                String retrievedRequest = session.getProvider(HttpClientProvider.class).getString(requestUri);
+                new ParEndpointRequestObjectParser(session, retrievedRequest, client).parseRequest(request);
             }
 
             if (Profile.isFeatureEnabled(Profile.Feature.DYNAMIC_SCOPES)) {

diff --git a/services/src/main/java/org/keycloak/protocol/oidc/utils/JWKSHttpUtils.java b/services/src/main/java/org/keycloak/protocol/oidc/utils/JWKSHttpUtils.java
@@ -17,14 +17,12 @@
 
 package org.keycloak.protocol.oidc.utils;
 
-import org.keycloak.common.util.StreamUtil;
 import org.keycloak.connections.httpclient.HttpClientProvider;
 import org.keycloak.jose.jwk.JSONWebKeySet;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.util.JsonSerialization;
 
 import java.io.IOException;
-import java.io.InputStream;
 
 /**
  *
@@ -33,9 +31,7 @@
 public class JWKSHttpUtils {
 
     public static JSONWebKeySet sendJwksRequest(KeycloakSession session, String jwksURI) throws IOException {
-        try (InputStream is = session.getProvider(HttpClientProvider.class).get(jwksURI)){
-            String keySetString = StreamUtil.readString(is);
-            return JsonSerialization.readValue(keySetString, JSONWebKeySet.class);
-        }
+        String keySetString = session.getProvider(HttpClientProvider.class).getString(jwksURI);
+        return JsonSerialization.readValue(keySetString, JSONWebKeySet.class);
     }
 }