[java-opensaml] branch main updated: OSJ-389: Use try-with-resources for HttpEntity access for HttpClient
Brent Putman
putmanb at georgetown.edu
Thu Aug 31 23:33:30 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=99c0c4bd9f63eb8ac3ce54089cbfd6bdd38ba157
The following commit(s) were added to refs/heads/main by this push:
new 99c0c4bd9 OSJ-389: Use try-with-resources for HttpEntity access for HttpClient
99c0c4bd9 is described below
commit 99c0c4bd9f63eb8ac3ce54089cbfd6bdd38ba157
Author: Brent Putman <putmanb at georgetown.edu>
AuthorDate: Thu Aug 31 19:32:23 2023 -0400
OSJ-389: Use try-with-resources for HttpEntity access for HttpClient
---
.../BaseHttpClientResponseXMLMessageDecoder.java | 13 ++---
.../impl/AbstractDynamicHTTPMetadataResolver.java | 20 ++++---
.../impl/AbstractReloadingMetadataResolver.java | 10 +---
.../resolver/impl/HTTPMetadataResolver.java | 10 +---
.../opensaml/soap/client/http/HttpSOAPClient.java | 19 ++++---
.../http/impl/HttpClientResponseSOAP11Decoder.java | 65 ++++++++++++----------
6 files changed, 68 insertions(+), 69 deletions(-)
diff --git a/opensaml-messaging-api/src/main/java/org/opensaml/messaging/decoder/httpclient/BaseHttpClientResponseXMLMessageDecoder.java b/opensaml-messaging-api/src/main/java/org/opensaml/messaging/decoder/httpclient/BaseHttpClientResponseXMLMessageDecoder.java
index a908792de..3154d78b5 100644
--- a/opensaml-messaging-api/src/main/java/org/opensaml/messaging/decoder/httpclient/BaseHttpClientResponseXMLMessageDecoder.java
+++ b/opensaml-messaging-api/src/main/java/org/opensaml/messaging/decoder/httpclient/BaseHttpClientResponseXMLMessageDecoder.java
@@ -14,6 +14,7 @@
package org.opensaml.messaging.decoder.httpclient;
+import java.io.IOException;
import java.io.InputStream;
import javax.annotation.Nonnull;
@@ -101,7 +102,7 @@ public abstract class BaseHttpClientResponseXMLMessageDecoder extends AbstractHt
try {
final Element dom = XMLObjectSupport.marshall(XMLObject.class.cast(message));
return SerializeSupport.prettyPrintXML(dom);
- } catch (MarshallingException e) {
+ } catch (final MarshallingException e) {
log.error("Unable to marshall message for logging purposes", e);
return null;
}
@@ -117,13 +118,9 @@ public abstract class BaseHttpClientResponseXMLMessageDecoder extends AbstractHt
* @throws MessageDecodingException thrown if there is a problem deserializing and unmarshalling the message
*/
protected XMLObject unmarshallMessage(@Nonnull final InputStream messageStream) throws MessageDecodingException {
- try {
- final XMLObject message = XMLObjectSupport.unmarshallFromInputStream(getParserPool(), messageStream);
- return message;
- } catch (final XMLParserException e) {
- log.error("Error unmarshalling message from input stream: {}", e.getMessage());
- throw new MessageDecodingException("Error unmarshalling message from input stream", e);
- } catch (final UnmarshallingException e) {
+ try (messageStream) {
+ return XMLObjectSupport.unmarshallFromInputStream(getParserPool(), messageStream);
+ } catch (final XMLParserException|UnmarshallingException|IOException e) {
log.error("Error unmarshalling message from input stream: {}", e.getMessage());
throw new MessageDecodingException("Error unmarshalling message from input stream", e);
}
diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/AbstractDynamicHTTPMetadataResolver.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/AbstractDynamicHTTPMetadataResolver.java
index ddb6d2122..5927a1c81 100644
--- a/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/AbstractDynamicHTTPMetadataResolver.java
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/AbstractDynamicHTTPMetadataResolver.java
@@ -31,6 +31,7 @@ import org.apache.hc.client5.http.classic.methods.HttpGet;
import org.apache.hc.client5.http.protocol.HttpClientContext;
import org.apache.hc.core5.http.ClassicHttpRequest;
import org.apache.hc.core5.http.ClassicHttpResponse;
+import org.apache.hc.core5.http.HttpEntity;
import org.apache.hc.core5.http.HttpStatus;
import org.apache.hc.core5.http.io.HttpClientResponseHandler;
import org.opensaml.core.xml.XMLObject;
@@ -354,14 +355,17 @@ public abstract class AbstractDynamicHTTPMetadataResolver extends AbstractDynami
return null;
}
- try {
- final InputStream ins = response.getEntity().getContent();
- final byte[] source = ByteStreams.toByteArray(ins);
- assert source != null;
- try (final ByteArrayInputStream bais = new ByteArrayInputStream(source)) {
- final XMLObject xmlObject = unmarshallMetadata(bais);
- xmlObject.getObjectMetadata().put(new XMLObjectSource(source));
- return xmlObject;
+ try (final HttpEntity entity = response.getEntity()) {
+ try (final InputStream ins = entity.getContent()) {
+ // TODO why are we buffering the InputStream to byte[] like this,
+ // rather than just unmarshalling it directly?
+ final byte[] source = ByteStreams.toByteArray(ins);
+ assert source != null;
+ try (final ByteArrayInputStream bais = new ByteArrayInputStream(source)) {
+ final XMLObject xmlObject = unmarshallMetadata(bais);
+ xmlObject.getObjectMetadata().put(new XMLObjectSource(source));
+ return xmlObject;
+ }
}
} catch (final IOException | UnmarshallingException e) {
log.error("{} Error unmarshalling HTTP response stream", getLogPrefix(), e);
diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/AbstractReloadingMetadataResolver.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/AbstractReloadingMetadataResolver.java
index 3815ebc49..439aa30ee 100644
--- a/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/AbstractReloadingMetadataResolver.java
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/AbstractReloadingMetadataResolver.java
@@ -682,7 +682,7 @@ public abstract class AbstractReloadingMetadataResolver extends AbstractBatchMet
* @throws ResolverException thrown if there is a problem reading the resultant byte array
*/
@Nonnull protected byte[] inputstreamToByteArray(@Nonnull final InputStream ins) throws ResolverException {
- try {
+ try (ins) {
// 1 MB read buffer
final byte[] buffer = new byte[1024 * 1024];
final ByteArrayOutputStream output = new ByteArrayOutputStream();
@@ -695,15 +695,7 @@ public abstract class AbstractReloadingMetadataResolver extends AbstractBatchMet
return output.toByteArray();
} catch (final IOException e) {
throw new ResolverException(e);
- } finally {
- try {
- ins.close();
- } catch (final IOException e) {
- // Ignore here. If the read() threw also, then that should be reported, not this.
- // If the close() throws, we don't care b/c we've already read the bytes.
- }
}
-
}
/** Background task that refreshes metadata. */
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 460a8d204..575a5a9e8 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
@@ -15,7 +15,6 @@
package org.opensaml.saml.metadata.resolver.impl;
import java.io.IOException;
-import java.io.InputStream;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.Timer;
@@ -29,6 +28,7 @@ import org.apache.hc.client5.http.protocol.HttpClientContext;
import org.apache.hc.core5.http.ClassicHttpRequest;
import org.apache.hc.core5.http.ClassicHttpResponse;
import org.apache.hc.core5.http.Header;
+import org.apache.hc.core5.http.HttpEntity;
import org.apache.hc.core5.http.HttpStatus;
import org.apache.hc.core5.http.io.entity.EntityUtils;
import org.opensaml.saml.metadata.resolver.RemoteMetadataResolver;
@@ -296,15 +296,11 @@ public class HTTPMetadataResolver extends AbstractReloadingMetadataResolver impl
throws ResolverException {
log.debug("{} Attempting to extract metadata from response to request for metadata from '{}'",
getLogPrefix(), getMetadataURI());
- try {
- final InputStream ins = response.getEntity().getContent();
- return inputstreamToByteArray(ins);
+ try (final HttpEntity entity = response.getEntity()) {
+ return inputstreamToByteArray(entity.getContent());
} catch (final IOException e) {
log.error("{} Unable to read response: {}", getLogPrefix(), e.getMessage());
throw new ResolverException("Unable to read response", e);
- } finally {
- // Make sure entity has been completely consumed.
- EntityUtils.consumeQuietly(response.getEntity());
}
}
}
\ No newline at end of file
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 6771addcd..7a2758159 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
@@ -290,11 +290,12 @@ public class HttpSOAPClient extends AbstractInitializableComponent implements SO
*/
protected void processSuccessfulResponse(@Nonnull final ClassicHttpResponse httpResponse,
@Nonnull final InOutOperationContext context) throws SOAPClientException {
- try {
- if (httpResponse.getEntity() == null) {
+ try (final HttpEntity entity = httpResponse.getEntity()) {
+ if (entity == null) {
throw new SOAPClientException("No response body from server");
}
- final Envelope response = unmarshallResponse(httpResponse.getEntity().getContent());
+
+ final Envelope response = unmarshallResponse(entity.getContent());
context.setInboundMessageContext(new MessageContext());
context.ensureInboundMessageContext().ensureSubcontext(SOAP11Context.class).setEnvelope(response);
//TODO: goes away?
@@ -315,11 +316,13 @@ public class HttpSOAPClient extends AbstractInitializableComponent implements SO
*/
protected void processFaultResponse(@Nonnull final ClassicHttpResponse httpResponse,
@Nonnull final InOutOperationContext context) throws SOAPClientException, SOAPFaultException {
- try {
- if (httpResponse.getEntity() == null) {
+
+ try (final HttpEntity entity = httpResponse.getEntity()){
+ if (entity == null) {
throw new SOAPClientException("No response body from server");
}
- final Envelope response = unmarshallResponse(httpResponse.getEntity().getContent());
+
+ final Envelope response = unmarshallResponse(entity.getContent());
context.setInboundMessageContext(new MessageContext());
context.ensureInboundMessageContext().ensureSubcontext(SOAP11Context.class).setEnvelope(response);
@@ -369,7 +372,7 @@ public class HttpSOAPClient extends AbstractInitializableComponent implements SO
*/
@Nonnull protected Envelope unmarshallResponse(@Nonnull final InputStream responseStream)
throws SOAPClientException {
- try {
+ try (responseStream) {
final Element responseElem = parserPool.parse(responseStream).getDocumentElement();
assert responseElem != null;
if (log.isDebugEnabled()) {
@@ -379,7 +382,7 @@ public class HttpSOAPClient extends AbstractInitializableComponent implements SO
XMLObjectProviderRegistrySupport.getUnmarshallerFactory().getUnmarshaller(responseElem),
"SOAP envelope unmarshaller not available");
return (Envelope) unmarshaller.unmarshall(responseElem);
- } catch (final XMLParserException e) {
+ } catch (final XMLParserException|IOException e) {
throw new SOAPClientException("Unable to parse the XML within the response", e);
} catch (final UnmarshallingException e) {
throw new SOAPClientException("Unable to unmarshall the response DOM", e);
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 fb3052bb4..526c61477 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
@@ -22,6 +22,7 @@ import javax.annotation.Nullable;
import javax.xml.namespace.QName;
import org.apache.hc.core5.http.ClassicHttpResponse;
+import org.apache.hc.core5.http.HttpEntity;
import org.apache.hc.core5.http.HttpStatus;
import org.opensaml.core.xml.XMLObject;
import org.opensaml.messaging.context.MessageContext;
@@ -157,19 +158,22 @@ public class HttpClientResponseSOAP11Decoder extends BaseHttpClientResponseXMLMe
@Nonnull final SOAP11Context soapContext)
throws MessageDecodingException, IOException {
- if (httpResponse.getEntity() == null) {
- throw new MessageDecodingException("No response body from server");
- }
- final Envelope soapMessage = (Envelope) unmarshallMessage(httpResponse.getEntity().getContent());
-
- // Defensive sanity check, otherwise body handler could later fail non-gracefully with runtime exception
- final Fault fault = getFault(soapMessage);
- if (fault != null) {
- throw new SOAP11FaultDecodingException(fault);
+ try (final HttpEntity entity = httpResponse.getEntity()) {
+ if (entity == null) {
+ throw new MessageDecodingException("No response body from server");
+ }
+
+ final Envelope soapMessage = (Envelope) unmarshallMessage(entity.getContent());
+
+ // Defensive sanity check, otherwise body handler could later fail non-gracefully with runtime exception
+ final Fault fault = getFault(soapMessage);
+ if (fault != null) {
+ throw new SOAP11FaultDecodingException(fault);
+ }
+
+ soapContext.setEnvelope(soapMessage);
+ soapContext.setHTTPResponseStatus(httpResponse.getCode());
}
-
- soapContext.setEnvelope(soapMessage);
- soapContext.setHTTPResponseStatus(httpResponse.getCode());
}
/**
@@ -184,24 +188,27 @@ public class HttpClientResponseSOAP11Decoder extends BaseHttpClientResponseXMLMe
@Nonnull protected MessageDecodingException buildFaultException(@Nonnull final ClassicHttpResponse response)
throws MessageDecodingException, IOException {
- if (response.getEntity() == null) {
- throw new MessageDecodingException("No response body from server");
- }
- final Envelope soapMessage = (Envelope) unmarshallMessage(response.getEntity().getContent());
-
- final Fault fault = getFault(soapMessage);
- if (fault == null) {
- throw new MessageDecodingException("HTTP status code was 500 but SOAP response did not contain a Fault");
+ try (final HttpEntity entity = response.getEntity()) {
+ if (entity == null) {
+ throw new MessageDecodingException("No response body from server");
+ }
+
+ final Envelope soapMessage = (Envelope) unmarshallMessage(entity.getContent());
+
+ final Fault fault = getFault(soapMessage);
+ if (fault == null) {
+ throw new MessageDecodingException("HTTP status code was 500 but SOAP response contained no Fault");
+ }
+
+ final FaultCode fcode = fault.getCode();
+ final QName code = fcode != null ? fcode.getValue() : null;
+
+ final FaultString fmsg = fault.getMessage();
+ final String msg = fmsg != null ? fmsg.getValue() : null;
+ log.debug("SOAP fault code '{}' with message '{}'", code != null ? code.toString() : "(not set)", msg);
+
+ return new SOAP11FaultDecodingException(fault);
}
-
- final FaultCode fcode = fault.getCode();
- final QName code = fcode != null ? fcode.getValue() : null;
-
- final FaultString fmsg = fault.getMessage();
- final String msg = fmsg != null ? fmsg.getValue() : null;
- log.debug("SOAP fault code '{}' with message '{}'", code != null ? code.toString() : "(not set)", msg);
-
- return new SOAP11FaultDecodingException(fault);
}
/**
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list