Configuring SameSite on a per-cookie basis in Jetty 12

[wilkinsona]

Jetty version

12.0.0.alpha3

Java version

17

Question

We'd like to support Jetty 12 in Spring Boot 3.1 and I am investigating the changes that we'll need to make. It's going pretty well but I've hit a problem with cookies and SameSite.

Spring Boot allows users to implement a strategy interface that can supply a cookie's SameSite value on a per-cookie basis. With Jetty 11 and earlier, we've implemented this functionality using a HandlerWrapper sub-class. Can you please give me some pointers on how this can be achieved using Handler.Wrapper in Jetty 12? Thus far, I have been unable to figure out how to intercept the addition of a cookie with the new API.

[joakime]

Will Spring Boot be using the low level handling in Jetty Core exclusively? or via Servlet from one of the ee levels?

[wilkinsona]

Thanks for taking a look, @joakime. The latter – we're embedding Jetty as a Servlet 6-compatible server so we're using the ee10 artifacts.

[joakime]

Then don't try to do this with Handler.Wrapper, as you've no doubt noticed, there's no Servlet anything in Jetty Core anymore.

It "should" work like this (haven't tried this personally) ...

• Start with overriding the org.eclipse.jetty.ee10.servlet.ServletContextHandler from the ee10 environment.
• You should focus on wrapping the ServletContextRequest from protected ServletContextRequest wrapRequest(Request request, Response response).
• A wrapper around the ServletContextResponse held by the ServletContextRequest is the ultimate goal you are looking for.
• Double check that the response from protected ContextResponse wrapResponse(ContextRequest request, Response response) is providing the wrapped response you are providing.

Something like this ...

        context = new ServletContextHandler() {
            @Override
            protected ServletContextRequest wrapRequest(Request request, Response response)
            {
                ServletContextRequest jettyRequest = super.wrapRequest(request, response);
                return new SpringServletContextRequest(jettyRequest);
            }
        };

[wilkinsona]

Thanks again, @joakime. Unfortunately, I think I've fallen at the second hurdle as I'm not sure how to wrap a ServletContextRequest. I assume I need to override org.eclipse.jetty.ee10.servlet.ServletContextHandler.wrap(Request) (I couldn't find a wrapRequest(Request, Response) method) so I'm looking at org.eclipse.jetty.ee10.servlet.ServletContextRequest which has a single 7-argument constructor. The first argument is a ServletContextHandler.ServletContextApi which doesn't seem to be available from the ServletContextRequest that I need to wrap. Apologies if I've missed something but I can't see how to create a sub-class of ServletContextRequest that wraps an existing ServletContextRequest.

[joakime]

Can you wait until 12.0.0.beta0 release to continue this effort?

[joakime]

@gregw please take a look at PR #9175 for proposed cleanup of ServletApiRequest and ServletApiResponse with an eye on allowing them to be wrapped.

[joakime]

@wilkinsona can you take a look at the following and see if this is what you had in mind?

• jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ServletContextHandlerTest#testCustomServletResponseWrapping
• on branch fix/jetty-12.0.x/custom-servlet-context-request-response-wrapping

[wilkinsona]

Can you wait until 12.0.0.beta0 release to continue this effort?

Yes, of course.

can you take a look at the following and see if this is what you had in mind?

That's exactly the sort of thing that I had in mind. Thank you. I'll look forward to beta0.

[gregw]

@sbordet @lachlan-roberts We've made addCookie a static method on Response so it can't be wrapped. I'm wondering if that is the best? I guess it goes in line with our policy of don't allow two ways to do something, else wrappers become very complex.
However, in this case, it means that in order to intercept cookies we would need to wrap the response in order to override getHeaders(), so that it could return a wrapped HttpFields so that it could intercept cookies!

Note also that wrapping servlet API requests/responses in ee10 is currently difficult to do as the the servlet API wrappers are created by the ServletContextHandler, but then the request/response are passed onwards as core request/responses, thus the intermediate handlers after the context cannot easily wrap the servlet API classes, at least not until servlet Filters, where we are fully into the Servlet API land.

So I think we need to have a think about how we can wrap easily in ee10 environment. This also relates to #9297

[gregw]

@wilkinsona Where are you with this issue at the moment? I'd like to make some significant changes to wrapping in ee10... probably between beta1 and beta2, so it would be good to know what you are currently doing and make sure we keep a good solution for you.

[wilkinsona]

Thanks for asking, @gregw. I parked my efforts in January after the discussion with @joakime. I haven't picked things up again, partly due to other priorities and partly due to waiting for the outcome of this issue.

What's the current rough timeline for Jetty 12? Spring Boot 3.1's RC is later this month (20 April) with GA the following month (18 May). Given the feature freeze that RC will bring, it's looking like support for Jetty 12 may become a Spring Boot 3.2 feature. That would push it back to November 2023 and hopefully give us plenty of time to thrash out the details.

[gregw]

We'd very much like to get support in for the RC. But the 20th is tough for us.
Give me a few days more to see if we can come up with a stable API that could be used for your RC.

[gregw]

I'm picking this up again (sorry for delay), so we don't miss November!

I think I'd like to think of this in general terms. Specifically it is difficult in Jetty-12 to intercept any response header, Cookies included. Currently the solution is to:

• write a Handler that wraps the response
• the response wrapper needs to wrap the HttpFields returned from Response.getHeaders
• the HttpFields wrapper needs to wrap at least the iterator and should also override the convenience methods for efficiency
• the HttpFields wrapper needs to override the mutation methods

That's a lot of wrapping and the result is multiple places that new headers need to be intercepted.

Option A
For the response to have a HttpField processor like:

    void addHttpFieldProcessor(Function<HttpField, HttpField> processor);

This could then be used in a Handler as follows

    @Override
    public boolean handle(Request request, Response response, Callback callback) throws Exception
    {
        response.addHttpFieldProcessor(field ->
        {
            if (field.getHeader() != HttpHeader.SET_COOKIE)
                return field;
            return new HttpField(HttpHeader.SET_COOKIE,
                field.getValue().replaceFirst("SameSite=[A-Za-z]+;", "SameSite=" + sameSiteFor(request) + ";"));
        });
        
        // ...
    }

The downside of this approach is that the Set-Cookie is already generated and has to be parsed before being mutated.
We'd also need to set expectation regarding headers already in the response (I guess they could be reprocessed?) and headers set/sent by the container during response generation.

Option B
To make the current static Response#addCookie(Response, HttpCookie method a non-static Response#addCookie(HttpCookie).

This will be simple to wrap/override, but the downside is that it will not intercept any cookies added by directly calling response.getHeaders().add(HttpField). Also it is back to a Cookie only solution, so intercepting other headers is still difficult.

[wilkinsona]

Thanks for picking this up again, @gregw. I'm not sure I know enough about Jetty at this level to have an informed opinion, but please don't hesitate to ask if there's something that you'd like me to try with Spring Boot to see if it meets our needs.

[sbordet]

@gregw while I think the approach of a HttpField processor works, I wonder if it would be better to leverage HttpChannel events, with the idea that Spring would implement the response "begin" event (just before commit), so that they will be able to do something like:

void onResponseBegin(Response response) 
{
    response.getHeaders().computeField(HttpHeader.SET_COOKIE, (header, fields) ->
    {
        ...
    });
}

Admittedly, it may require a bit of string parsing and/or concatenation, but it is a rare case to require this feature.

This approach would make things much simpler, avoid the overhead in the API (not need for the addHttpFieldProcessor() method) and in the implementation.

Thoughts?

[gregw]

@sbordet i think that works. It's no less efficient than the HttpFields approach if we keep the changes i make in the PR to cache the cookie in a field type

[wilkinsona]

Thanks very much for the changes here. I now have a branch where Spring Boot's SameSite cookie support works with Jetty 12.0.0.beta2.