[java-support] 02/02: IDP-1793 Use Suppliers for HttpRequest/Response

Rod Widdowson rdw at steadingsoftware.com
Sat Aug 6 15:10:29 UTC 2022


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

rdw pushed a commit to branch maint-8
in repository java-support.

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

commit cab99b2f4da30bb920bd998f22626432f42134c3
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Sat Aug 6 16:06:09 2022 +0100

    IDP-1793  Use Suppliers for HttpRequest/Response
    
    https://shibboleth.atlassian.net/browse/IDP-1793
    
    The CookieManager gains setters for a Supplier<HttpServletRequest>
    and Response.  The raw setter is retained, but is desprecated
    and just injects a supplier which is used throughout the implementation.
---
 .../utilities/java/support/net/CookieManager.java  | 113 +++++++++++++++++----
 .../java/support/net/CookieManagerTest.java        |  42 ++++++--
 2 files changed, 129 insertions(+), 26 deletions(-)

diff --git a/src/main/java/net/shibboleth/utilities/java/support/net/CookieManager.java b/src/main/java/net/shibboleth/utilities/java/support/net/CookieManager.java
index 6ee7927..40541b9 100644
--- a/src/main/java/net/shibboleth/utilities/java/support/net/CookieManager.java
+++ b/src/main/java/net/shibboleth/utilities/java/support/net/CookieManager.java
@@ -17,19 +17,26 @@
 
 package net.shibboleth.utilities.java.support.net;
 
+import java.util.function.Supplier;
+
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import javax.servlet.http.Cookie;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
 import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
 import net.shibboleth.utilities.java.support.component.AbstractInitializableComponent;
 import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
 import net.shibboleth.utilities.java.support.component.ComponentSupport;
 import net.shibboleth.utilities.java.support.logic.Constraint;
+import net.shibboleth.utilities.java.support.primitive.DeprecationSupport;
 import net.shibboleth.utilities.java.support.primitive.StringSupport;
+import net.shibboleth.utilities.java.support.primitive.DeprecationSupport.ObjectType;
 
 /**
  * A helper class for managing one or more cookies on behalf of a component.
@@ -40,17 +47,20 @@ import net.shibboleth.utilities.java.support.primitive.StringSupport;
  */
 public final class CookieManager extends AbstractInitializableComponent {
 
+    /** Log. */
+    private final Logger log = LoggerFactory.getLogger(CookieManager.class);
+
     /** Path of cookie. */
     @Nullable private String cookiePath;
 
     /** Domain of cookie. */
     @Nullable private String cookieDomain;
     
-    /** Servlet request to read from. */
-    @NonnullAfterInit private HttpServletRequest httpRequest;
+    /** Supplier for the servlet request to read from. */
+    @NonnullAfterInit private Supplier<HttpServletRequest> httpRequestSupplier;
 
-    /** Servlet response to write to. */
-    @NonnullAfterInit private HttpServletResponse httpResponse;
+    /** Supplier for the servlet response to write to. */
+    @NonnullAfterInit private Supplier<HttpServletResponse> httpResponseSupplier;
     
     /** Is cookie secure? */
     private boolean secure;
@@ -95,29 +105,95 @@ public final class CookieManager extends AbstractInitializableComponent {
     }
 
     /**
-     * Set the servlet request to read from.
-     * 
-     * @param request servlet request
+     * Set the Supplier for the servlet request to read from.
+     *
+     * @param requestSupplier servlet request supplier
      */
-    public void setHttpServletRequest(@Nonnull final HttpServletRequest request) {
+    public void setHttpServletRequestSupplier(@Nonnull final Supplier<HttpServletRequest> requestSupplier) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
         ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
         
-        httpRequest = Constraint.isNotNull(request, "HttpServletRequest cannot be null");
+        httpRequestSupplier = Constraint.isNotNull(requestSupplier, "HttpServletRequest cannot be null");
     }
 
     /**
-     * Set the servlet response to write to.
-     * 
-     * @param response servlet response
+     * Set the current HTTP request.
+     *
+     * @param request current HTTP request
      */
-    public void setHttpServletResponse(@Nonnull final HttpServletResponse response) {
+    @Deprecated(since = "4.3", forRemoval = true)
+    public void setHttpServletRequest(@Nullable final HttpServletRequest request) {
+        ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+        DeprecationSupport.warnOnce(ObjectType.METHOD, "setHttpServletReqest",
+                "CookieManager", "setHttpServletRequestSupplier");
+        if (request != null && !(request instanceof ThreadLocalHttpServletRequestProxy)) {
+            log.warn("Unsafe HttpServletRequest injected");
+        }
+        httpRequestSupplier = new Supplier<>() {
+            public HttpServletRequest get() {
+                return request;
+            };
+        };
+    }
+
+    /**
+     * Get the current HTTP request if available.
+     *
+     * @return current HTTP request
+     */
+    @NonnullAfterInit private HttpServletRequest getHttpServletRequest() {
+        if (httpRequestSupplier == null) {
+            return null;
+        }
+        return httpRequestSupplier.get();
+    }
+
+    /**
+     * Set the supplier for the servlet response to write to.
+     *
+     * @param responseSupplier servlet response
+     */
+    public void setHttpServletResponseSupplier(@Nonnull final Supplier<HttpServletResponse> responseSupplier) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
         ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
         
-        httpResponse = Constraint.isNotNull(response, "HttpServletResponse cannot be null");
+        httpResponseSupplier = Constraint.isNotNull(responseSupplier, "HttpServletResponse cannot be null");
     }
 
+    /**
+     * Set the servlet response to write to.
+     *
+     * @param response current HTTP response
+     */
+    @Deprecated(since = "4.3", forRemoval = true)
+    public void setHttpServletResponse(@Nullable final HttpServletResponse response) {
+        ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+
+        DeprecationSupport.warnOnce(ObjectType.METHOD, "setHttpServletResponse",
+                "CookieManager", "setHttpServletResponseSupplier");
+        if (response != null && !(response instanceof ThreadLocalHttpServletResponseProxy)) {
+            log.warn("Unsafe HttpServletRequest injected");
+        }
+        httpResponseSupplier = new Supplier<>() {
+            public HttpServletResponse get() {
+                return response;
+            };
+        };
+    }
+
+    /**
+     * Get the current HTTP response if available.
+     *
+     * @return current HTTP response or null
+     */
+    @NonnullAfterInit private HttpServletResponse getHttpServletResponse() {
+        if (httpResponseSupplier == null) {
+            return null;
+        }
+        return httpResponseSupplier.get();
+    }
+
+
     /**
      * Set the SSL-only flag.
      * 
@@ -159,7 +235,7 @@ public final class CookieManager extends AbstractInitializableComponent {
     protected void doInitialize() throws ComponentInitializationException {
         super.doInitialize();
         
-        if (httpRequest == null || httpResponse == null) {
+        if (getHttpServletRequest() == null || getHttpServletResponse() == null) {
             throw new ComponentInitializationException("Servlet request and response must be set");
         }
     }
@@ -183,7 +259,7 @@ public final class CookieManager extends AbstractInitializableComponent {
         cookie.setHttpOnly(httpOnly);
         cookie.setMaxAge(maxAge);
         
-        httpResponse.addCookie(cookie);
+        getHttpServletResponse().addCookie(cookie);
     }
 
     /**
@@ -204,7 +280,7 @@ public final class CookieManager extends AbstractInitializableComponent {
         cookie.setHttpOnly(httpOnly);
         cookie.setMaxAge(0);
         
-        httpResponse.addCookie(cookie);
+        getHttpServletResponse().addCookie(cookie);
     }
 
     /**
@@ -237,7 +313,7 @@ public final class CookieManager extends AbstractInitializableComponent {
         ComponentSupport.ifNotInitializedThrowUninitializedComponentException(this);
         ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
         
-        final Cookie[] cookies = httpRequest.getCookies();
+        final Cookie[] cookies = getHttpServletRequest().getCookies();
         if (cookies != null) {
             for (final Cookie cookie : cookies) {
                 if (cookie.getName().equals(name)) {
@@ -255,6 +331,7 @@ public final class CookieManager extends AbstractInitializableComponent {
      * @return  the cookie path
      */
     @Nonnull @NotEmpty private String contextPathToCookiePath() {
+        final  HttpServletRequest httpRequest = getHttpServletRequest();
         return "".equals(httpRequest.getContextPath()) ? "/" : httpRequest.getContextPath();
     }
     
diff --git a/src/test/java/net/shibboleth/utilities/java/support/net/CookieManagerTest.java b/src/test/java/net/shibboleth/utilities/java/support/net/CookieManagerTest.java
index 5215f3b..86578de 100644
--- a/src/test/java/net/shibboleth/utilities/java/support/net/CookieManagerTest.java
+++ b/src/test/java/net/shibboleth/utilities/java/support/net/CookieManagerTest.java
@@ -17,7 +17,11 @@
 
 package net.shibboleth.utilities.java.support.net;
 
+import java.util.function.Supplier;
+
 import javax.servlet.http.Cookie;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 
 import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
 
@@ -44,8 +48,8 @@ public class CookieManagerTest {
         MockHttpServletResponse response = new MockHttpServletResponse();
         
         CookieManager cm = new CookieManager();
-        cm.setHttpServletRequest(request);
-        cm.setHttpServletResponse(response);
+        cm.setHttpServletRequestSupplier(new Supplier<>() { public HttpServletRequest get() {return request;}});
+        cm.setHttpServletResponseSupplier(new Supplier<>() { public HttpServletResponse get() {return response;}});
         cm.initialize();
     }
 
@@ -53,14 +57,35 @@ public class CookieManagerTest {
         MockHttpServletRequest request = new MockHttpServletRequest();
         MockHttpServletResponse response = new MockHttpServletResponse();
         
+        CookieManager cm = new CookieManager();
+        cm.setHttpServletRequestSupplier(new Supplier<>() { public HttpServletRequest get() {return request;}});
+        cm.setHttpServletResponseSupplier(new Supplier<>() { public HttpServletResponse get() {return response;}});
+        cm.setCookiePath("/idp");
+        cm.initialize();
+
+        cm.addCookie("foo", "bar");
+
+        Cookie cookie = response.getCookie("foo");
+        Assert.assertNotNull(cookie);
+        Assert.assertEquals(cookie.getValue(), "bar");
+        Assert.assertEquals(cookie.getPath(), "/idp");
+        Assert.assertNull(cookie.getDomain());
+        Assert.assertTrue(cookie.getSecure());
+        Assert.assertEquals(cookie.getMaxAge(), -1);
+    }
+    
+    @Test public void testCookieWithPathOldSetters() throws ComponentInitializationException {
+        MockHttpServletRequest request = new MockHttpServletRequest();
+        MockHttpServletResponse response = new MockHttpServletResponse();
+        
         CookieManager cm = new CookieManager();
         cm.setHttpServletRequest(request);
         cm.setHttpServletResponse(response);
         cm.setCookiePath("/idp");
         cm.initialize();
-        
+
         cm.addCookie("foo", "bar");
-        
+
         Cookie cookie = response.getCookie("foo");
         Assert.assertNotNull(cookie);
         Assert.assertEquals(cookie.getValue(), "bar");
@@ -70,14 +95,15 @@ public class CookieManagerTest {
         Assert.assertEquals(cookie.getMaxAge(), -1);
     }
 
+
     @Test public void testCookieNoPath() throws ComponentInitializationException {
         MockHttpServletRequest request = new MockHttpServletRequest();
         request.setContextPath("/idp");
         MockHttpServletResponse response = new MockHttpServletResponse();
         
         CookieManager cm = new CookieManager();
-        cm.setHttpServletRequest(request);
-        cm.setHttpServletResponse(response);
+        cm.setHttpServletRequestSupplier(new Supplier<>() { public HttpServletRequest get() {return request;}});
+        cm.setHttpServletResponseSupplier(new Supplier<>() { public HttpServletResponse get() {return response;}});
         cm.initialize();
         
         cm.addCookie("foo", "bar");
@@ -98,8 +124,8 @@ public class CookieManagerTest {
         MockHttpServletResponse response = new MockHttpServletResponse();
         
         CookieManager cm = new CookieManager();
-        cm.setHttpServletRequest(request);
-        cm.setHttpServletResponse(response);
+        cm.setHttpServletRequestSupplier(new Supplier<>() { public HttpServletRequest get() {return request;}});
+        cm.setHttpServletResponseSupplier(new Supplier<>() { public HttpServletResponse get() {return response;}});
         cm.initialize();
         
         cm.unsetCookie("foo");

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


More information about the commits mailing list