jsr369-experts@servlet-spec.java.net

[jsr369-experts] Re: [servlet-spec users] Re: Re: [SERVLET_SPEC-134] Push API vJetty

From: Stuart Douglas <sdouglas_at_redhat.com>
Date: Tue, 18 Aug 2015 18:04:59 -0400 (EDT)

----- Original Message -----
> From: "Shing Wai Chan" <shing.wai.chan_at_oracle.com>
> To: jsr369-experts_at_servlet-spec.java.net
> Sent: Wednesday, 19 August, 2015 7:57:16 AM
> Subject: [servlet-spec users] [jsr369-experts] Re: Re: [SERVLET_SPEC-134] Push API vJetty
>
> Besides queryString, other http headers also have a similar issue.
> So, we can do one of the following (as in queryString):
> a) remove the initialized for headers
> b) add removeHeader API

I think removeHeader makes sense. Unlike the query string the automatically added headers are likely to be wanted in the pushed response.

Stuart

>
> Any comments?
> Thanks.
> Shing Wai Chan
>
> On 8/17/15, 3:52 PM, Stuart Douglas wrote:
> >
> > ----- Original Message -----
> >> From: "Shing Wai Chan"<shing.wai.chan_at_oracle.com>
> >> To: jsr369-experts_at_servlet-spec.java.net
> >> Cc: "Stuart Douglas"<sdouglas_at_redhat.com>
> >> Sent: Tuesday, 18 August, 2015 2:48:42 AM
> >> Subject: [jsr369-experts] Re: [servlet-spec users] Re: [SERVLET_SPEC-134]
> >> Push API vJetty
> >>
> >> However, from the javadoc, the queryString is "initialized as follows":
> >> * The query string from HttpServletRequest.getQueryString().
> >>
> >> So, this implies that the path() call inherits the query string from the
> >> original request.
> >> That is why I ask the question.
> >>
> >> To fix the issue, we can do one of the following:
> >> a) remove the initialized for queryString from above
> >> b) add remove/replace queryString API
> >>
> >> For the queryString issue, (a) is better.
> >> But then we also need to consider similar issues for headers.
> >
> > Oops, I totally miss read that.
> >
> > I don't think that it makes any sense to initialise the query string to be
> > the same as the original request. I think in the majority of cases the new
> > request will have a different query string, so I think your option a) is
> > the way to go.
> >
> > Stuart
> >
> >
> >
> >> Thanks.
> >> Shing Wai Chan
> >>
> >> On 8/16/15, 6:01 PM, Stuart Douglas wrote:
> >>> ----- Original Message -----
> >>>> From: "Shing Wai Chan"<shing.wai.chan_at_oracle.com>
> >>>> To: jsr369-experts_at_servlet-spec.java.net
> >>>> Sent: Monday, 17 August, 2015 2:33:19 AM
> >>>> Subject: [jsr369-experts] Re: [servlet-spec users] [SERVLET_SPEC-134]
> >>>> Push
> >>>> API vJetty
> >>>>
> >>>> From javadoc,
> >>>> "The instance is initialized as follows:
> >>>> ...
> >>>> * The query string from HttpServletRequest.getQueryString()
> >>>>
> >>>> PushBuilder queryString(String queryString)
> >>>> Set the query string to be used for the push. Will be appended to any
> >>>> query String included in a call to path(String).
> >>>> "
> >>>>
> >>>> So, this means we can only add more query String, but cannot modify or
> >>>> remove the query string from the httpServletRequest.
> >>>> Is it an issue here? Do we need APIs to remove or clean up or replace
> >>>> the query string?
> >>>> (Just consider the case of push a jpg file which may/do not need query
> >>>> string. Or in some case, we need to "replace" the query string.)
> >>>> (The same may be for other headers.)
> >>> This is the query string from the .path() call, not the query string from
> >>> the original
> >>> request. I think in this case it makes sense, if you don't want a query
> >>> string just don't
> >>> include one in the call to path().
> >>>
> >>> Stuart
> >>>
> >>>> Shing Wai Chan
> >>>>
> >>>>
> >>>> On 8/14/15, 4:09 PM, Edward Burns wrote:
> >>>>>>>>>> On Wed, 12 Aug 2015 09:16:19 +1000, Greg
> >>>>>>>>>> Wilkins<gregw_at_webtide.com>
> >>>>>>>>>> said:
> >>>>> GW> We've not done a lot of work or analysis on this
> >>>>> GW> recently.... however it is included in our stable release and
> >>>>> here
> >>>>> GW> is the API that we are currently providing:
> >>>>>
> >>>>> GW>
> >>>>> https://github.com/eclipse/jetty.project/blob/master/jetty-server/src/main/java/org/eclipse/jetty/server/PushBuilder.java
> >>>>>
> >>>>> Thanks Greg,
> >>>>>
> >>>>> I've taken this as a starting point and created a branch on which we
> >>>>> can
> >>>>> iterate. I've published the artifact and the javadoc is at [1].
> >>>>>
> >>>>> I've added getPushBuilder() to HttpServletRequest [2].
> >>>>>
> >>>>> I've reworded your text somewhat, and am still in the process of doing
> >>>>> that. The biggest and only normative change to what you had is in the
> >>>>> final sentance of the class javadoc for PushBuilder. I've called out
> >>>>> some specific areas for discussion here.
> >>>>>
> >>>>> A PushBuilder can be customized by chained calls to mutator
> >>>>> methods
> >>>>> before the push() method is called to *initiate an asynchronous
> >>>>> push
> >>>>> request with the current state of the builder.* After the call to
> >>>>> push(), the builder may be reused for another push, *however the
> >>>>> implementation must make it so the path(String), etag(String) and
> >>>>> lastModified(String) values are cleared before returning from
> >>>>> push()*. All other values are retained over calls to push().
> >>>>>
> >>>>> GW> However, while it is being used, I would not characterize this
> >>>>> as
> >>>>> GW> extensively tested or analysed. To my knowledge nobody has used
> >>>>> the
> >>>>> API
> >>>>> GW> directly in a framework, but some have deployed the push filter
> >>>>> in
> >>>>> their
> >>>>> GW> applications.
> >>>>>
> >>>>> GW> Note that while I like the idea of the simple degenerate case,
> >>>>> my
> >>>>> limited
> >>>>> GW> experience is that building the synthetic request is actually a
> >>>>> lot
> >>>>> more
> >>>>> GW> complex than you'd think and there are lots of corner/edge
> >>>>> cases.
> >>>>> Even the
> >>>>> GW> PushBuilder needs to synthesize/validate headers and the rules
> >>>>> for
> >>>>> that
> >>>>> GW> will need to be carefully crafted and explained to achieve
> >>>>> portability.
> >>>>>
> >>>>> You know, I think I can live without the dispatcher that I showed at
> >>>>> GeekOut on slide 70 of [3]:
> >>>>>
> >>>>> It could just as well be
> >>>>>
> >>>>> ((HttpServletRequest)request).getRequestBuilder().path(url).push();
> >>>>>
> >>>>> Let's get some discussion going here!
> >>>>>
> >>>>> Ed
> >>>>>
>