qos-ch/logback-access

Implement getCookies, getLocalPort, and getLocalAddr methods in RequestWrapper

Opened this issue · 8 comments

Logback Access 2.0.1 and Jetty 12.

The methods getCookies, getLocalPort, and getLocalAddr currently exist within the RequestWrapper class but do not return meaningful data as one might expect.

These methods will be used in my project and I expect to improve them.
Moreover, the improvements to getCookies and getLocalPort methods are useful as they correlate with the values used in logback-access's pattern layouts, such as %reqCookie for cookies and %localPort for the local port. (For some reason, %localIP works without the getLocalAddr implementation.

I propose the following changes:

  • getCookies(): This method should be improved to convert and return an array of Cookie objects, derived from the List<HttpCookie> returned by Request::getCookies.
  • getLocalPort(): By using Request::getLocalPort, this method could be improved to return the port number.
  • getLocalAddr(): Similarly by using Request::getLocalAddr, this method could be improved to return the local address.
ceki commented

Thank you for your comments. I am looking forward to your PRs.

Please add units tests where possible.

ceki commented

Created a jira issue for secondary tracking ACCCESS-1

Thank you. I'll try it.

I found a issue while considering tests for getCookies().

ceki commented

Fixed in commits 8509ec2 and 0d300d8

@ceki Thank you so much. But, I have a exception.

java.lang.ClassCastException: class [Ljava.lang.Object; cannot be cast to class [Ljakarta.servlet.http.Cookie; ([Ljava.lang.Object; is in module java.base of loader 'bootstrap'; [Ljakarta.servlet.http.Cookie; is in module jakarta.servlet@6.0.0 of loader 'app')
        at ch.qos.logback.access.jetty@2.0.2-SNAPSHOT/ch.qos.logback.access.jetty.RequestWrapper.getCookies(RequestWrapper.java:68)
        at ch.qos.logback.access.common@2.0.2-SNAPSHOT/ch.qos.logback.access.common.spi.AccessEvent.getCookies(AccessEvent.java:474)
        at ch.qos.logback.access.common@2.0.2-SNAPSHOT/ch.qos.logback.access.common.spi.AccessEvent.getCookie(AccessEvent.java:482)

I would suggest using toArray with a type instead.

diff --git a/jetty12/src/main/java/ch/qos/logback/access/jetty/RequestWrapper.java b/jetty12/src/main/java/ch/qos/logback/access/jetty/RequestWrapper.java
index b5d390c..309a441 100644
--- a/jetty12/src/main/java/ch/qos/logback/access/jetty/RequestWrapper.java
+++ b/jetty12/src/main/java/ch/qos/logback/access/jetty/RequestWrapper.java
@@ -65,7 +65,7 @@ public class RequestWrapper implements HttpServletRequest, WrappedHttpRequest {
         List<Cookie> cookieList = httpCookies.stream().map(httpCookie -> new Cookie(httpCookie.getName(), httpCookie.getValue())).collect(
                 Collectors.toList());
 
-        return (Cookie[]) cookieList.toArray();
+        return cookieList.toArray(new Cookie[cookieList.size()]);
     }
 
     @Override

All three are working fine in logback-access version 2.0.2.
Thank you for the release.

@ceki It looks like this issue has been resolved, so could you please process the closing.
Or should the closing process be done by myself, the issue author?