[java-shib-shared] branch main updated: JSSH-25 - HTTPResource needs some rework to properly close response

[Scott Cantor]

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

scantor pushed a commit to branch main
in repository java-shib-shared.

View the commit online:
http://git.shibboleth.net/view/?p=java-shib-shared.git;a=commit;h=6879bfa40bdd6fbf7972b0efd64a81836d30c5a8

The following commit(s) were added to refs/heads/main by this push:
     new 6879bfa4 JSSH-25 - HTTPResource needs some rework to properly close response
6879bfa4 is described below

commit 6879bfa40bdd6fbf7972b0efd64a81836d30c5a8
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Tue Mar 14 08:51:17 2023 -0400

    JSSH-25 - HTTPResource needs some rework to properly close response
    
    https://shibboleth.atlassian.net/browse/JSSH-25
    
    Small adjustment to eliminate legacy class.
    Added TODO on questionable method.
---
 .../spring/httpclient/resource/HTTPResource.java       | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/shib-networking-spring/src/main/java/net/shibboleth/shared/spring/httpclient/resource/HTTPResource.java b/shib-networking-spring/src/main/java/net/shibboleth/shared/spring/httpclient/resource/HTTPResource.java
index 4599b471..b390a705 100644
--- a/shib-networking-spring/src/main/java/net/shibboleth/shared/spring/httpclient/resource/HTTPResource.java
+++ b/shib-networking-spring/src/main/java/net/shibboleth/shared/spring/httpclient/resource/HTTPResource.java
@@ -33,7 +33,6 @@ import org.apache.hc.client5.http.cache.HttpCacheContext;
 import org.apache.hc.client5.http.classic.HttpClient;
 import org.apache.hc.client5.http.classic.methods.HttpGet;
 import org.apache.hc.client5.http.classic.methods.HttpUriRequest;
-import org.apache.hc.client5.http.impl.classic.CloseableHttpResponse;
 import org.apache.hc.client5.http.utils.DateUtils;
 import org.apache.hc.core5.http.ClassicHttpResponse;
 import org.apache.hc.core5.http.Header;
@@ -251,6 +250,8 @@ public class HTTPResource extends AbstractIdentifiedInitializableComponent imple
      * Attempts to fetch only the headers for a given resource. If HEAD requests are unsupported then a more costly GET
      * request is performed.
      * 
+     * TODO: I suspect this needs to be revised to not return the HttpResponse since it's been closed.
+     * 
      * @return the response from the request
      * 
      * @throws IOException thrown if there is a problem contacting the resource
@@ -261,7 +262,6 @@ public class HTTPResource extends AbstractIdentifiedInitializableComponent imple
         ClassicHttpResponse httpResponse = null;
         try {
             final HttpCacheContext context = buildHttpClientContext();
-            
             final HttpClientContextHandler contextHandler = httpClientContextHandler;
             
             if (contextHandler != null) {
@@ -386,10 +386,10 @@ public class HTTPResource extends AbstractIdentifiedInitializableComponent imple
      * 
      * @param response the HTTP response
      */
-    protected void closeResponse(@Nullable final HttpResponse response) {
+    protected void closeResponse(@Nullable final ClassicHttpResponse response) {
         try {
-            if (response != null && response instanceof CloseableHttpResponse) {
-                ((CloseableHttpResponse) response).close();
+            if (response != null) {
+                response.close();
             }
         } catch (final IOException e) {
             log.error("Error closing HTTP response from '{}'", resourceURL.toExternalForm(), e);
@@ -403,10 +403,10 @@ public class HTTPResource extends AbstractIdentifiedInitializableComponent imple
      private static class ConnectionClosingInputStream extends InputStream {
 
         /** HTTP response that is being wrapped. */
-        private final ClassicHttpResponse response;
+        @Nonnull private final ClassicHttpResponse response;
 
         /** Stream owned by the given HTTP response. */
-        private final InputStream stream;
+        @Nonnull private final InputStream stream;
 
         /**
          * Constructor.
@@ -429,9 +429,7 @@ public class HTTPResource extends AbstractIdentifiedInitializableComponent imple
         @Override
         public void close() throws IOException {
             stream.close();
-            if (response instanceof CloseableHttpResponse) {
-               ((CloseableHttpResponse)response).close();
-            }
+            response.close();
         }
 
         /** {@inheritDoc} */

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