Login | Register
My pages Projects Community openCollabNet

Discussions > dev > UnsecureServlet method use unclear

Project highlights: Architectural Overview

joist
Discussion topic

Hide all messages in topic

All messages in topic

Re: [joist-dev] UnsecureServlet method use unclear

Author dlr
Full name Daniel Rall
Date 2000-10-10 09:53:41 PDT
Message Jon Stevens wrote:
>
> on 10/9/2000 8:41 PM, "David Pellegrini" <davidp at collab dot net> wrote:
>
> > Don't rush to judgement without understanding what this is doing.
> >
> > It's not doing a full browser redirect. In fact, this is to _avoid_ a full
> > redirect. So instead of using a HttpServletResponse.​sendRedirect(aURL), using
> > UnsecureServlet.redi​rectTo(servletName) allows for effectively "redirecting"
> > to another servlet, all in the same request, and with the context intact. No
> > need to construct a URL complete with query string arguments and pay the
> > performance penalty of the round trip to the browser.
> >
> > Raising an exception to accomplish this greatly reduced the code complexity,
> > enough so to outweigh any performance impact IMO. You can still do the usual
> > redirects using a convenience method UnsecureServlet.redi​rectToURL(theContext​,
> > aURL).
> >
> > -davidp
>
> Of course I understand what is going on here. Turbine has the same exact
> feature, but it don't use an exception to perform this process. There are
> two methods:
>
> doRedirect(data, "templateName")
> and
> setTemplate(data, "templateName")
>
> In the first case, it will execute both the template and the associated Java
> class with the template (associated by name) in order to build the context.
>
> In the second case, it will execute the template only with the current
> context. Neither method uses an exception to make things happen.
>
> Please don't take this as a "Turbine is better" or "Switch to Turbine" or
> whatever. I'm simply explaining that there are other ways to get the same
> functionality without throwing an exception.
>
> If the feature is added to Joist to remove the getServlet() method (which
> needs to be done anyway), then it will be easy to re-use that same code to
> re-implement the doRedirect() method with that code instead.

Sounds like a better solution.
--

Daniel Rall <dlr at finemaltcoding dot com>

Re: [joist-dev] UnsecureServlet method use unclear

Author Jon Stevens <jon at latchkey dot com>
Full name Jon Stevens <jon at latchkey dot com>
Date 2000-10-10 09:47:54 PDT
Message on 10/9/2000 8:41 PM, "David Pellegrini" <davidp at collab dot net> wrote:

> Don't rush to judgement without understanding what this is doing.
>
> It's not doing a full browser redirect. In fact, this is to _avoid_ a full
> redirect. So instead of using a HttpServletResponse.​sendRedirect(aURL), using
> UnsecureServlet.redi​rectTo(servletName) allows for effectively "redirecting"
> to another servlet, all in the same request, and with the context intact. No
> need to construct a URL complete with query string arguments and pay the
> performance penalty of the round trip to the browser.
>
> Raising an exception to accomplish this greatly reduced the code complexity,
> enough so to outweigh any performance impact IMO. You can still do the usual
> redirects using a convenience method UnsecureServlet.redi​rectToURL(theContext​,
> aURL).
>
> -davidp

Of course I understand what is going on here. Turbine has the same exact
feature, but it don't use an exception to perform this process. There are
two methods:

doRedirect(data, "templateName")
and
setTemplate(data, "templateName")

In the first case, it will execute both the template and the associated Java
class with the template (associated by name) in order to build the context.

In the second case, it will execute the template only with the current
context. Neither method uses an exception to make things happen.

Please don't take this as a "Turbine is better" or "Switch to Turbine" or
whatever. I'm simply explaining that there are other ways to get the same
functionality without throwing an exception.

If the feature is added to Joist to remove the getServlet() method (which
needs to be done anyway), then it will be easy to re-use that same code to
re-implement the doRedirect() method with that code instead.

-jon

--
http://scarab.tigris.org/ | http://noodle.tigris.org/
http://java.apache.org/ | http://java.apache.org/turbine/
http://www.working-dogs.com/ | http://jakarta.apach​e.org/velocity/
http://www.collab.net/ | http://www.sourcexchange.com/

Re: [joist-dev] UnsecureServlet method use unclear

Author "Daniel L dot Rall" <dlr at collab dot net>
Full name "Daniel L dot Rall" <dlr at collab dot net>
Date 2000-10-09 20:47:10 PDT
Message David Pellegrini wrote:
>
> Jon Stevens wrote:
> >
> > on 10/9/2000 9:46 AM, "Daniel L. Rall" <dlr at finemaltcoding dot com> wrote:
> >
> > > Why is an Exception even thrown if you just want a redirect? To quote
> > > Brian Kernighan, "exceptions should be used only in exceptional
> > > circumstances." This doesn't qualify. Additionally, this method means
> > > nothing on its own, which makes its purpose indecipherable without
> > > looking at how it's called. Just today you mentioned that Exceptions
> > > are not cheap to create, so why bother when what you actually want is a
> > > redirect?
> > >
> > > Daniel Rall
> >
> > I agree. This is a bad design decision.
>
> Don't rush to judgement without understanding what this is doing.
>
> It's not doing a full browser redirect. In fact, this is to _avoid_ a full redirect. So instead of using a HttpServletResponse.​sendRedirect(aURL), using UnsecureServlet.redi​rectTo(servletName) allows for effectively "redirecting" to another servlet, all in the same request, and with the context intact. No need to construct a URL complete with query string arguments and pay the performance penalty of the round trip to the browser.

No one said it was doing a browser redirect. I understand what the
exception is doing, and I dislike the implementation.

> Raising an exception to accomplish this greatly reduced the code complexity, enough so to outweigh any performance impact IMO. You can still do the usual redirects using a convenience method UnsecureServlet.redi​rectToURL(theContext​, aURL).

Okay. Will you document your weirdness when you have a moment, please?
This is not something that should be left unnoted. Thanks.
--

Daniel Rall <dlr at collab dot net>
http://collab.net/ | open source | do the right thing

Re: [joist-dev] UnsecureServlet method use unclear

Author David Pellegrini <davidp at collab dot net>
Full name David Pellegrini <davidp at collab dot net>
Date 2000-10-09 20:41:44 PDT
Message Jon Stevens wrote:
>
> on 10/9/2000 9:46 AM, "Daniel L. Rall" <dlr at finemaltcoding dot com> wrote:
>
> > Why is an Exception even thrown if you just want a redirect? To quote
> > Brian Kernighan, "exceptions should be used only in exceptional
> > circumstances." This doesn't qualify. Additionally, this method means
> > nothing on its own, which makes its purpose indecipherable without
> > looking at how it's called. Just today you mentioned that Exceptions
> > are not cheap to create, so why bother when what you actually want is a
> > redirect?
> >
> > Daniel Rall
>
> I agree. This is a bad design decision.

Don't rush to judgement without understanding what this is doing.

It's not doing a full browser redirect. In fact, this is to _avoid_ a full redirect. So instead of using a HttpServletResponse.​sendRedirect(aURL), using UnsecureServlet.redi​rectTo(servletName) allows for effectively "redirecting" to another servlet, all in the same request, and with the context intact. No need to construct a URL complete with query string arguments and pay the performance penalty of the round trip to the browser.

Raising an exception to accomplish this greatly reduced the code complexity, enough so to outweigh any performance impact IMO. You can still do the usual redirects using a convenience method UnsecureServlet.redi​rectToURL(theContext​, aURL).

-davidp

Re: [joist-dev] UnsecureServlet method use unclear

Author Ed Korthof <edk at laswell dot collab dot net>
Full name Ed Korthof <edk at laswell dot collab dot net>
Date 2000-10-09 17:26:28 PDT
Message On Mon, Oct 09, 2000 at 04:58:19PM -0700, Jon Stevens wrote:
> on 10/9/2000 9:46 AM, "Daniel L. Rall" <dlr at finemaltcoding dot com> wrote:
>
> > Why is an Exception even thrown if you just want a redirect? To quote
> > Brian Kernighan, "exceptions should be used only in exceptional
> > circumstances." This doesn't qualify. Additionally, this method means
> > nothing on its own, which makes its purpose indecipherable without
> > looking at how it's called. Just today you mentioned that Exceptions
> > are not cheap to create, so why bother when what you actually want is a
> > redirect?
> >
> > Daniel Rall
>
> I agree. This is a bad design decision.

I don't like it (I agree that it's a bad design decision), but I don't
think it's so obvious that this is a misuse of exceptions. If we assume
that you might realize you want to issue a redirect in a function other
than processRequest -- in fact, in a function far down the stack from
processRequest -- then exceptions are the right approach : without them,
you have to complicate the code significantly to deal with early return
after (or before) doing the redirect.

(Redirects themselves happen infrequently (once per request == infrequent)
-- so the cost of creating an exception isn't all that big a deal. This
is another situation where I'd say that if we want to optimize, we should
start with operations which are costing us *a lot*. I don't think this
is one of them. If you want, I can do some tests to figure out the time
it takes to throw & catch an exception; I suspect it'll be somewhat less
than 1ms, which puts it beneath the threshold I'd call relevant, given
that our servlets are taking ~250ms or more total time.)

OTOH -- IMO, servlets should be very simple, and the logic required for
an early return shouldn't be too complicated. I don't think the code
involved in choosing to make an early return should be in the servlet
itself -- I think it should be in the next level down. If the code is
well organized, exceptions could be avoided without excessive
complication. Right now, I think the code may be organized in such a way
that switching to a system which doesn't work by exceptions would be
painful and would create far too much complication. :-(

The way I see it, the basic question is more how we should encourage
people to organize their servlets/logic. This looks like a valid use of
exceptions, but it leads to code which is more complicated (at each
level).

cheers --

Ed
--
   +=-=+=-=+=-=+=-=+=-=​+=-=+=-=+=-=+=-=+=-=​+=-=+=-=+=-=+=-=
   | Ed Korthof | edk at collab dot net | 415-247-1690 |
   +=-=+=-=+=-=+=-=+=-=​+=-=+=-=+=-=+=-=+=-=​+=-=+=-=+=-=+=-=

Re: [joist-dev] UnsecureServlet method use unclear

Author Jon Stevens <jon at latchkey dot com>
Full name Jon Stevens <jon at latchkey dot com>
Date 2000-10-09 16:58:19 PDT
Message on 10/9/2000 9:46 AM, "Daniel L. Rall" <dlr at finemaltcoding dot com> wrote:

> Why is an Exception even thrown if you just want a redirect? To quote
> Brian Kernighan, "exceptions should be used only in exceptional
> circumstances." This doesn't qualify. Additionally, this method means
> nothing on its own, which makes its purpose indecipherable without
> looking at how it's called. Just today you mentioned that Exceptions
> are not cheap to create, so why bother when what you actually want is a
> redirect?
>
> Daniel Rall

I agree. This is a bad design decision.

-jon

--
http://scarab.tigris.org/ | http://noodle.tigris.org/
http://java.apache.org/ | http://java.apache.org/turbine/
http://www.working-dogs.com/ | http://jakarta.apach​e.org/velocity/
http://www.collab.net/ | http://www.sourcexchange.com/

UnsecureServlet method use unclear

Author dlr
Full name Daniel Rall
Date 2000-10-09 15:14:30 PDT
Message I don't understand the point of this method:

    /**
     * Redirect to another page
     *
     * @param targetPage the page to which to redirect
     * @exception RedirectException
     */
    protected void
    redirectTo (String targetPage)
    throws RedirectException
    {
    if (debug) log.debug("redirectTo: " + targetPage);
    throw new RedirectServletExcep​tion(targetPage);
    }


It does nothing but throw a RedirectServletException.

Daniel

Re: [joist-dev] UnsecureServlet method use unclear

Author dlr
Full name Daniel Rall
Date 2000-10-09 09:46:36 PDT
Message David Pellegrini wrote:
>
> dlr at finemaltcoding dot com wrote:
> >
> > I don't understand the point of this method:
> >
> > /**
> > * Redirect to another page
> > *
> > * @param targetPage the page to which to redirect
> > * @exception RedirectException
> > */
> > protected void
> > redirectTo (String targetPage)
> > throws RedirectException
> > {
> > if (debug) log.debug("redirectTo: " + targetPage);
> > throw new RedirectServletExcep​tion(targetPage);
> > }
> >
> > It does nothing but throw a RedirectServletException.
>
> ... specifying the name of the servlet to redirect to. The catch clause
> for this exception pulls the name of the servlet from the exception,
> then gets an instance of the servlet by that name, then invokes handle()
> on that servlet instance. This is the basis of the "internal redirect".

Why is an Exception even thrown if you just want a redirect? To quote
Brian Kernighan, "exceptions should be used only in exceptional
circumstances." This doesn't qualify. Additionally, this method means
nothing on its own, which makes its purpose indecipherable without
looking at how it's called. Just today you mentioned that Exceptions
are not cheap to create, so why bother when what you actually want is a
redirect?

Daniel Rall

Re: [joist-dev] UnsecureServlet method use unclear

Author David Pellegrini <davidp at collab dot net>
Full name David Pellegrini <davidp at collab dot net>
Date 2000-10-09 09:28:22 PDT
Message dlr at finemaltcoding dot com wrote:
>
> I don't understand the point of this method:
>
> /**
> * Redirect to another page
> *
> * @param targetPage the page to which to redirect
> * @exception RedirectException
> */
> protected void
> redirectTo (String targetPage)
> throws RedirectException
> {
> if (debug) log.debug("redirectTo: " + targetPage);
> throw new RedirectServletExcep​tion(targetPage);
> }
>
> It does nothing but throw a RedirectServletException.

... specifying the name of the servlet to redirect to. The catch clause
for this exception pulls the name of the servlet from the exception,
then gets an instance of the servlet by that name, then invokes handle()
on that servlet instance. This is the basis of the "internal redirect".

-davidp
Messages per page: