[java-opensaml] branch main updated: OSJ-387: Always use try-with-resources with HttpClient requests ...

Brent Putman putmanb at georgetown.edu
Wed Aug 30 02:51:27 UTC 2023


This is an automated email from the git hooks/post-receive script.

putmanb pushed a commit to branch main
in repository java-opensaml.

View the commit online:
http://git.shibboleth.net/view/?p=java-opensaml.git;a=commit;h=813b8ce7cd038746c44402aea06bbe1d8b4fff4c

The following commit(s) were added to refs/heads/main by this push:
     new 813b8ce7c OSJ-387: Always use try-with-resources with HttpClient requests ...
813b8ce7c is described below

commit 813b8ce7cd038746c44402aea06bbe1d8b4fff4c
Author: Brent Putman <putmanb at georgetown.edu>
AuthorDate: Tue Aug 29 22:27:28 2023 -0400

    OSJ-387: Always use try-with-resources with HttpClient requests ...
    
    Always use try-with-resources with HttpClient requests not involving
    HttpClientResponseHandler
---
 .../resolver/impl/HTTPMetadataResolver.java        | 15 ++------
 .../http/AbstractPipelineHttpSOAPClient.java       | 19 +++++-----
 .../opensaml/soap/client/http/HttpSOAPClient.java  | 42 +++++++---------------
 .../http/impl/HttpClientResponseSOAP11Decoder.java |  8 -----
 4 files changed, 25 insertions(+), 59 deletions(-)

diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/HTTPMetadataResolver.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/HTTPMetadataResolver.java
index 9f2295920..460a8d204 100644
--- a/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/HTTPMetadataResolver.java
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/HTTPMetadataResolver.java
@@ -196,11 +196,10 @@ public class HTTPMetadataResolver extends AbstractReloadingMetadataResolver impl
     @Nullable protected byte[] fetchMetadata() throws ResolverException {
         final HttpGet httpGet = buildHttpGet();
         final HttpClientContext context = buildHttpClientContext(httpGet);
-        ClassicHttpResponse response = null;
 
-        try {
-            log.debug("{} Attempting to fetch metadata document from '{}'", getLogPrefix(), metadataURI);
-            response = httpClient.executeOpen(null, httpGet, context);
+        log.debug("{} Attempting to fetch metadata document from '{}'", getLogPrefix(), metadataURI);
+        try (final ClassicHttpResponse response = httpClient.executeOpen(null, httpGet, context)) {
+            
             HttpClientSecuritySupport.checkTLSCredentialEvaluated(context, metadataURI.getScheme());
             final int httpStatusCode = response.getCode();
 
@@ -228,14 +227,6 @@ public class HTTPMetadataResolver extends AbstractReloadingMetadataResolver impl
             final String errMsg = "Error retrieving metadata from " + metadataURI;
             log.error("{} {}: {}", getLogPrefix(), errMsg, e.getMessage());
             throw new ResolverException(errMsg, e);
-        } finally {
-            try {
-                if (response != null) {
-                    response.close();
-                }
-            } catch (final IOException e) {
-                log.error("{} Error closing HTTP response from {}", metadataURI, getLogPrefix(), e);
-            }
         }
     }
 
diff --git a/opensaml-soap-api/src/main/java/org/opensaml/soap/client/http/AbstractPipelineHttpSOAPClient.java b/opensaml-soap-api/src/main/java/org/opensaml/soap/client/http/AbstractPipelineHttpSOAPClient.java
index 1a03e7b4a..68120219f 100644
--- a/opensaml-soap-api/src/main/java/org/opensaml/soap/client/http/AbstractPipelineHttpSOAPClient.java
+++ b/opensaml-soap-api/src/main/java/org/opensaml/soap/client/http/AbstractPipelineHttpSOAPClient.java
@@ -196,15 +196,16 @@ public abstract class AbstractPipelineHttpSOAPClient
             
             // HttpClient execution
             final HttpClientContext httpContext = buildHttpContext(httpRequest, operationContext);
-            final ClassicHttpResponse httpResponse = getHttpClient().executeOpen(null, httpRequest, httpContext);
-            HttpClientSecuritySupport.checkTLSCredentialEvaluated(httpContext, httpRequest.getScheme());
-            
-            // Response decoding
-            final HttpClientResponseMessageDecoder decoder = pipeline.getDecoder();
-            decoder.setHttpResponse(httpResponse);
-            decoder.initialize();
-            decoder.decode();
-            operationContext.setInboundMessageContext(decoder.getMessageContext());
+            try (final ClassicHttpResponse httpResponse = getHttpClient().executeOpen(null, httpRequest, httpContext)) {
+                HttpClientSecuritySupport.checkTLSCredentialEvaluated(httpContext, httpRequest.getScheme());
+
+                // Response decoding
+                final HttpClientResponseMessageDecoder decoder = pipeline.getDecoder();
+                decoder.setHttpResponse(httpResponse);
+                decoder.initialize();
+                decoder.decode();
+                operationContext.setInboundMessageContext(decoder.getMessageContext());
+            }
             
             // Inbound message handling
             
diff --git a/opensaml-soap-api/src/main/java/org/opensaml/soap/client/http/HttpSOAPClient.java b/opensaml-soap-api/src/main/java/org/opensaml/soap/client/http/HttpSOAPClient.java
index 56c91c65a..6771addcd 100644
--- a/opensaml-soap-api/src/main/java/org/opensaml/soap/client/http/HttpSOAPClient.java
+++ b/opensaml-soap-api/src/main/java/org/opensaml/soap/client/http/HttpSOAPClient.java
@@ -33,6 +33,7 @@ import org.apache.hc.core5.http.ContentType;
 import org.apache.hc.core5.http.HttpEntity;
 import org.apache.hc.core5.http.HttpStatus;
 import org.apache.hc.core5.http.io.entity.ByteArrayEntity;
+import org.apache.hc.core5.http.io.entity.EntityUtils;
 import org.opensaml.core.xml.XMLObject;
 import org.opensaml.core.xml.config.XMLObjectProviderRegistrySupport;
 import org.opensaml.core.xml.io.Marshaller;
@@ -189,7 +190,6 @@ public class HttpSOAPClient extends AbstractInitializableComponent implements SO
                 Constraint.isNotNull(strategy, "SOAP 1.1 context lookup strategy cannot be null");
     }
     
-// Checkstyle: CyclomaticComplexity OFF
     /** {@inheritDoc} */
     public void send(@Nonnull @NotEmpty final String endpoint, @Nonnull final InOutOperationContext context)
             throws SOAPException, SecurityException {
@@ -208,42 +208,24 @@ public class HttpSOAPClient extends AbstractInitializableComponent implements SO
             soapRequestParams = (HttpSOAPRequestParameters) clientCtx.getSOAPRequestParameters();
         }
         
-        HttpPost post = null;
-        try {
-            post = createPostMethod(endpoint, soapRequestParams, env);
+        final HttpPost post = createPostMethod(endpoint, soapRequestParams, env);
 
-            ClassicHttpResponse response = null;
-            try {
-                response = httpClient.executeOpen(null, post, null);
-                final int code = response.getCode();
-                log.debug("Received HTTP status code of {} when POSTing SOAP message to {}", code, endpoint);
+        try (final ClassicHttpResponse response = httpClient.executeOpen(null, post, null)) {
+            final int code = response.getCode();
+            log.debug("Received HTTP status code of {} when POSTing SOAP message to {}", code, endpoint);
 
-                if (code == HttpStatus.SC_OK) {
-                    processSuccessfulResponse(response, context);
-                } else if (code == HttpStatus.SC_INTERNAL_SERVER_ERROR) {
-                    processFaultResponse(response, context);
-                } else {
-                    throw new SOAPClientException("Received " + code +
-                            " HTTP response status code from HTTP request to " + endpoint);
-                }
-            } finally {
-                try {
-                    if (response != null) {
-                        response.close();
-                    }
-                } catch (final IOException e) {
-                    log.error("Error closing HttpResponse", e);
-                }
+            if (code == HttpStatus.SC_OK) {
+                processSuccessfulResponse(response, context);
+            } else if (code == HttpStatus.SC_INTERNAL_SERVER_ERROR) {
+                processFaultResponse(response, context);
+            } else {
+                throw new SOAPClientException("Received " + code +
+                        " HTTP response status code from HTTP request to " + endpoint);
             }
         } catch (final IOException e) {
             throw new SOAPClientException("Unable to send request to " + endpoint, e);
-        } finally {
-            if (post != null) {
-                post.reset();
-            }
         }
     }
-// Checkstyle: CyclomaticComplexity ON
 
     /**
      * Create the post method used to send the SOAP request.
diff --git a/opensaml-soap-impl/src/main/java/org/opensaml/soap/client/soap11/decoder/http/impl/HttpClientResponseSOAP11Decoder.java b/opensaml-soap-impl/src/main/java/org/opensaml/soap/client/soap11/decoder/http/impl/HttpClientResponseSOAP11Decoder.java
index ee59facdb..fb3052bb4 100644
--- a/opensaml-soap-impl/src/main/java/org/opensaml/soap/client/soap11/decoder/http/impl/HttpClientResponseSOAP11Decoder.java
+++ b/opensaml-soap-impl/src/main/java/org/opensaml/soap/client/soap11/decoder/http/impl/HttpClientResponseSOAP11Decoder.java
@@ -126,14 +126,6 @@ public class HttpClientResponseSOAP11Decoder extends BaseHttpClientResponseXMLMe
         } catch (final IOException e) {
             log.error("Unable to obtain input stream from HttpResponse: {}", e.getMessage());
             throw new MessageDecodingException("Unable to obtain input stream from HttpResponse", e);
-        } finally {
-            if (response != null) {
-                try {
-                    response.close();
-                } catch (final IOException e) {
-                    log.warn("Error closing HttpResponse", e);
-                }
-            }
         }
         
         try {

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list