-
-
Notifications
You must be signed in to change notification settings - Fork 754
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
resource.getRequest().getCookies() doesn't work #485
Comments
It's websocket, right? |
yep - websocket. |
I'm still seeing this on 1.0.1, Tomcat 7.0.30, Windows 7 64 bit. Using Websockets and looking for the cookies the first thing that happens in
|
I don't think there's a problem with that unit test, but it tests a manually populated cookie, not a list set by the server. I think that the problem might be that Tomcat does not give the cookies as expected, so there is no cookies available from the start. The request seems to be set as a HttpServletRequest, maybe it needs to get the WsHttpServletRequestWrapper - I don't know the inner workings and just use a bit of Google. So I'm not sure I can make a unit test, sorry. As a side note, wouldn't this code add the cookies again and again each time it's called (provided there were any cookies in the array :)) @Override
public Cookie[] getCookies() {
Cookie[] c = b.request.getCookies();
if (c != null && c.length > 0) {
b.cookies.addAll(Arrays.asList(c));
}
return b.cookies.toArray(new Cookie[]{});
} |
I just wanted to note that I've been running into this exact same issue (even all the way up to version 1.0.2). My setup is pretty simple: Chrome -> jquery.atmosphere.js -> Websocket -> Tomcat 7 (7.0.30) One thing I was a little confused on is why in the cloneRequest method we are always calling this code: b.request(r); which effectively replaces the real request with a NoOpsRequest so the call to request.getCookies(); returns this: @Override
public Cookie[] getCookies() {
return new Cookie[0];
} from the NoOpsRequest leaving us with what appears to be no cookies. Is there any way around this? I noticed that the JettyWebSocketUtil passes false (for loadInMemory) into the cloneRequest method so we'd never be replacing the request like this. Any tips you can provide for how to solve this would be great. Thanks! |
Also, the unit test you linked above doesn't test the Tomcat7Websocket path and also doesn't include any of the Websocket headers when building up the request so it doesn't get to the problem code: if (loadInMemory) {
r = new NoOpsRequest();
if (isWrapped) {
load(b.request, b);
} else {
load(request, b);
}
b.request(r);
} Hopefully I am just really misunderstanding/misconfiguring something here and this will be an easy fix. |
@toddwest The cookie are loaded in memory by doing Cookie[] cs = request.getCookies();
Set<Cookie> hs = new HashSet();
for (Cookie c: cs) {
hs.add(c);
}
b.cookies(hs); In AtmosphereRequest. I've re-opened the issue to see what can be done. |
Thanks for the reply! Yeah, I noticed that if I put a breakpoint right before this code: b.request(r); and make a call to request.getCookies(); it returns the full set of Cookies correctly. However if I make that call after that line of code it returns 0 cookies due to the NoOpsRequest replacing the real request. Look forward to seeing what you can find out! Thanks again! |
Wow, thank you for the quick fix! I really appreciate it! |
Very nice! Will this get pushed to Maven anytime soon you think? (It's how we use the library). |
@kribblo It is as a SNAPSHOT and I will release 1.0.3 today to tomorrow. Thanks! |
Perfect! Thanks! |
This seems to be happening again on Tomcat 8, as well as with Jetty 9.x on the latest version of atmosphere-runtime. From what I have seen I feel like anyone who is going through the JSR356AsyncSupport -> JSR356Endpoint is not getting cookies, but is getting a header with the cookies as a string. |
It is happening again, I have been trying to also read request's attributes I set in a Filter before reaching a Managed Service, and none of them are found inside the resource's AtmosphereRequest. Could this issue be related, or is a normal behavior that Websocket transport drops these attributes before reaching the Managed Service? BTW I found that someone already reported the cookies issue again: https://groups.google.com/forum/embed/?place=forum/atmosphere-framework&showsearch=true&showpopout=true&showtabs=false&parenturl=http%3A%2F%2Fasync-io.org%2Fdownload.html#!topic/atmosphere-framework/qbicJoaKmpU Regards |
Should this be reopened or file a new issue? |
@jaltman7 Reopen a new one ... with a fix :-) |
tested with version 1.0.0.beta2a (0.9.4 also) .
resource.getRequest().getCookies() - returns empty.
resource.getRequest().getHeader("cookie") - return the cookies (works)
The text was updated successfully, but these errors were encountered: