Issues with CookieHandler about vertx-web

[aliakhtar]

The cookie issue seems to be an old bug in the CookieDecoder class being used: netty/netty#649

the problem is that Chrome does not send expires back to the server when requesting.

[aliakhtar]

In M2, it seems like we're on an old version of netty: 4.0.24.Final . We should upgrade to
4.0.25.Final . The CookieDecoder class that's currently being used, is not present in netty trunk at all. It seems they became aware of the problems with it.

[purplefox]

Yes, we should change the cookiehandler to only write cookies back if they have changed

[aliakhtar]

The problem with knowing if they've changed, is that the browser only sends back the name and value, it doesn't send the path or expiry / max-age.

So we can determine if the name / value have changed, but if the expiry / path have changed, we won't be able to tell that. (If a user is logging out, the cookie would be deleted by setting max-age to a negative value).

I think a better solution is to provide a readonly Collection<Cookie> for reading the cookies, and store a separate List<Cookie> for the ones which are added via context.addCookie. That way, we only send back the ones which are added to the List and don't touch the readonly ones.

Would you like a PR that does that?

[aliakhtar]

Actually, a simpler solution would be to store a boolean isDirty flag in CookieImpl. At the start of the request, cookie.setDirty(false) is called on each cookie which is read from the request. Using any of the setters on the cookie would reset isDirty to true. Then, at the end, any cookies which weren't initially present in the map, or which have the isDirty set to true, are added back to response.

How does that sound?

[formalizator]

@aliakhtar Sounds good. This will take users off the annoyng copying back and forward.
But how the user will be able to get unmodified request-headers?

We can only modify response headers without affecting request ones.

[purplefox]

I've made some changes in the socksjs branch (just waiting for vietj to review it, then I can merge it in ;) )

[aliakhtar]

Looks nice. At this line though: https://github.com/vert-x3/vertx-apex/blob/ac7cd0b4152f1daf53986ca350535549389fc48f/src/main/java/io/vertx/ext/apex/impl/CookieImpl.java#L36

You're setting changed=true within the constructor. Wouldn't this mean that when the CookieHandler reads the cookies from the response, the cookies would be generated with changed=true even if they aren't modified?

Perhaps have a setChanged method present in there, so that CookieHandler can set it to false after first reading the cookie, and only subsequent modifications switch it back on.

[purplefox]

That constructor is only used before you add a new cookie, in which case you want it to be sent out in the response.