on 7/2/01 5:25 PM, "Peter Lynch" <[EMAIL PROTECTED]> wrote:
> In the turbine servlet there is code that does a
> redirect if getRedirectURI() != null.
>
> I would think that immediately after that, there
> whould be a "return;" as the next few lines before the
> end of the of try block are not intended to be
> executed, I think. The lines are...
>
> // Set the status code.
#1. > data.getResponse().setStatus ( data.getStatusCode() );
> // Output the Page.
#2. > data.getPage().output (data.getOut());
>
> Putting a "return;" does not prevent the finally{} of
> the try block from executing if that is what was
> thought by not putting it there. So the RunData will
> get added back to the cache regardless.
>
> What was intended by not putting a "return;" here and
> is it a potential bug?
>
> -Peter
I totally see where you are coming from, but wish to discuss this a bit
further...(notice that I made a #1 and #2 above)...
There is a case where a redirect header will get ignored by older (2.x?)
browsers. In that case, it is a good idea to output a HTML page with a link
in it so that people can click on it to continue.
In either case, #1, must be executed so that one can control the type of
status response (301 or 302).
Is the in-efficiency of #2 in the case of a redirect really worth anything?
Personally, I don't think so because it allows us to still have the feature
to output content.
Having said that, here is the patch that would do what you want. Move the
setStatus() to the top and then only output content if it isn't a redirect.
Index: org/apache/turbine/Turbine.java
===================================================================
RCS file:
/home/cvs/jakarta-turbine/src/java/org/apache/turbine/Turbine.java,v
retrieving revision 1.61
diff -r1.61 Turbine.java
563a564,566
> // Set the status code.
> data.getResponse().setStatus ( data.getStatusCode() );
>
573,577c576,580
<
< // Set the status code.
< data.getResponse().setStatus ( data.getStatusCode() );
< // Output the Page.
< data.getPage().output (data.getOut());
---
> else
> {
> // Output the Page.
> data.getPage().output (data.getOut());
> }
I'm not going to apply this to CVS quite yet because I think that the
feature of being able to output content is worth the minor in-efficiency in
a redirect (which is already highly inefficient and in most cases shouldn't
be used anyway).
Thanks,
-jon
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
