I've been working on adding push support to Tomcat and thinking a lot
about the PushBuilder. There are a number of issues in the e-mail below.
I've kept them together for now but we can always spin some or all of
them out to separate threads if necessary.
Issue 1: Consider changing how getPushBuilder() works.
I think there is merit in adding - in a similar way to
isAsyncSupported() - a new method:
isPushSupported()
and modifying the definition of getPushBuilder() to throw an ISE if
isPushSupported() returns false and getPushBuilder() is called.
This would mean removing the NO-OP PushBuilder.
This has several advantages:
- it aligns the push API with the async API;
- it allows applications to avoid the (possibly costly) process
of configuring a PushBuilder that is never going to do
anything.
There is one complication with this in that in HTTP/2 the client can
enable/disable server push at will. The server will need to handle this.
That may still mean some push requests are constructed and then thrown away.
Issue 2: Naming of methods
Why are some of the set() methods not named setXxxxx()?
Issue 3: Header manipulation
The getHeader() method can't handle multiple values for the same header.
Do we need something more like the HttpServletRequest interface?
How does getHeader() behave if the specified header does not exist?
Is the return value of getHeaderNames() a copy or backed by the data in
the PushBuilder? Is it modifiable?
It would be worth adding a note that HTTP header names must be handled
in a case insensitive manner.
Issue 4: Path encoding
The edge case I have in mind here is how do we handle a push request for
a static resource that has a '?' (or any other reserved character) in
the file name?
I think the answer is that the value passed to setPath() should use %nn
encoding where appropriate and that the same character encoding should
be used to decode it as was used for the original request URI.
Issue 5: Scheme and authority
Should scheme and authority be copied from source to target. It is
implied in the definition of setPath() but it would be better to be
explicit.
Issue 6: Cookies
Removing cookies with maxAge <= 0 is problematic. The request cookies
only have name/value pairs. No path is available. There is no way to
determine which cookie in the response maps to which cookie in the
request if there are multiple cookies with the same name but different
paths. How should be handle this?
Apologies for the fairly lengthy e-mail.
Mark