Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement getCookies, getLocalPort, and getLocalAddr methods in RequestWrapper #6

Open
marinedayo opened this issue Apr 12, 2024 · 6 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@marinedayo
Copy link
Contributor

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
Copy link
Member

ceki commented Apr 12, 2024

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

Please add units tests where possible.

@ceki
Copy link
Member

ceki commented Apr 12, 2024

Created a jira issue for secondary tracking ACCCESS-1

@ceki ceki self-assigned this Apr 12, 2024
@ceki ceki added bug Something isn't working enhancement New feature or request labels Apr 12, 2024
@marinedayo
Copy link
Contributor Author

Thank you. I'll try it.

@marinedayo
Copy link
Contributor Author

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

@ceki
Copy link
Member

ceki commented Apr 27, 2024

Fixed in commits 8509ec2 and 0d300d8

@marinedayo
Copy link
Contributor Author

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants