deprecating HttpRequest#requestURI & HttpRequest#contextPath

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

deprecating HttpRequest#requestURI & HttpRequest#contextPath

soft_props
I'm working on a module that requires access to the requests raw query string. With the current HttpRequest interface and its bindings I am not able to do that generically without reaching down into the underlying request which couples my code with the underlying implementation.

I am proposing and implementing on a separate branch the following changes

I wish to deprecate HttpRequest#requestURI and HttpRequest#contextPath then adding a supplemental HttpRequest#uri

As things sit now, HttpRequest's interface is really catering needlessly the Java's HttpServlet spec by adding a contextPath which has no meaning under Netty's http interface.

Java's servlet request interface method getRequestURI() chomps of the query string part of the request URI which Netty's does not. A query string is really part of the protocols requestURI so it is a little annoying that the way you'd actually get a request uri is to append Java's getQueryString, if it's not null, to getRequestURI. Instead of adding yet another method to the Unfiltered request interface to suit the servlet spec, I'm going to add a generic method called uri which, for netty, just returns Netty's underlying request.getUri, and for the servlet binding append getRequestURI and getQueryString, hopefully making things a little more consistent. I will then provide a proper extractor to get a raw query string using the generic unfiltered request interface method, uri.

The result of the above refactorings `should` have no impact on client code unless you are using a bindings #contextPath explicitly. It should only require some refactoring of the base extractors that have to do with paths.

An open question is if I should exclude the contextPath from the http servlet's request binding or not?

If I do not, your path extractors will have to account for it in order to have a successful match.

svr.context("/foo") { _.filter(Planify { case Path("/foo/bar") => Ok }) }

Currently, you do not have to do this because the Path extractor removes the contextPath for you. The resulting client code looks like

svr.context("/foo") { _.filter(Planify { case Path("/bar") => Ok }) }

This puts less of a burden on the developer and is more correct in that you should be able to `mount` your filter anywhere without changing your path extractors. This would be the result of not appending context path to the servlet request's binding for the uri method.

If you absolutely need need access to the context path, you will always have access to the underlying request. I hate having it be part of the base request interface and have it not be applicable to the Netty binding.

The goal would be to deprecate HttpRequest interface methods requestURI and contextPath for the next release, then remove them in the following release.

Let me know what you think.
Reply | Threaded
Open this post in threaded view
|

Re: deprecating HttpRequest#requestURI & HttpRequest#contextPath

n8han
Administrator
On 05/14/2011 04:34 PM, soft_props [via Databinder] wrote:
> An open question is if I should exclude the contextPath from the http
> servlet's request binding or not?

The other issue is that if contextPath is excluded from the binding, it
is not possible to implement HMAC request signature checking without
referring to the underlying (servlet) library. If HttpRequest is unaware
of servlet contexts, as we would like for it to be, it therefore has to
give back the full request url and break the context model.

The question is, is that okay with everybody? If not, we can sadly keep
HttpRequest context-aware, *or* (just had this idea) we could provide a
servlet-specific alternative to unfiltered.request.Path that is itself
context-aware.

Nathan
Reply | Threaded
Open this post in threaded view
|

Re: deprecating HttpRequest#requestURI & HttpRequest#contextPath

soft_props
I like that idea!

Have the filter request bind include the context path

but provide a filter module specific Path extractor

unfiltered.filter.request.Path which can be aware of the underlying request and chomp the context path off there

To get the full path you can use the default unfiltered.request.Path

+1

-Doug Tangren
http://lessis.me


On Sat, May 14, 2011 at 4:51 PM, n8han [via Databinder] <[hidden email]> wrote:
idea

Reply | Threaded
Open this post in threaded view
|

Re: deprecating HttpRequest#requestURI & HttpRequest#contextPath

n8han
Administrator
On 05/14/2011 05:08 PM, soft_props [via Databinder] wrote:
> Have the filter request bind include the context path
>
> but provide a filter module specific Path extractor
>
> unfiltered.filter.request.Path which can be aware of the underlying
> request and chomp the context path off there

Or, ContextPath:

     case ContextPath(context, path @ Seg(...)) =>

I think something along these lines is a good compromise. But we'll have
to put a big notice in the release notes for anybody that is using
servlet contexts; they'll have to change all their uses of Path.

Nathan
Reply | Threaded
Open this post in threaded view
|

Re: deprecating HttpRequest#requestURI & HttpRequest#contextPath

soft_props

Sounds good.

- Doug

Reply | Threaded
Open this post in threaded view
|

Re: deprecating HttpRequest#requestURI & HttpRequest#contextPath

soft_props