tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Costin Manolache <cmanola...@yahoo.com>
Subject Re: Question About DefaultCMSetter.java
Date Thu, 21 Sep 2000 15:36:28 GMT
Hi Larry,


> I am trying to determine if my assumption about ExceptionHandler and StatusHandler not
including "<html>" and "<body>" tags, while the others do, is significant.  My
recent changes to ContextManager.java in tomcat_32 assumed that it was.  As a result, ContextManager.handleException()
doesn't call res.resetBuffers() if ExceptionHandler is going to be used, to avoid clearing
"<html>" and "<body>" tags that may already be present.  I also modified handleStatus()
to do the same if StatusHandler is going to be used.

To be honest - I don't know :-)

It's the result of refactoring and trying to preserve existing behavior.

For StatusHandler - I think we should have <html> and <body>, initially it had
no body ( and I copied from ExceptionHandler ).

Exceptions shouldn't happend ( :-) ), and it's a taste decision if you
want an error page ( even if the error happened in included fragments)
or you want the error displayed in the place of fragment.

I think the first option is better ( the fragment may be invisible, etc),
errors should be clearly visible. AFAIK the current ( and previous
behavior ) used the second option.
One reason for keeping the current status - what happen if the
error happens after some output was commited ?



One thing that I know it's important is making the exception handler
configurable to disable stack traces !!! It's very bad for a production
site.

> Is this assumption valid, or should ExceptionHandler and StatusHandler be including the
"<html>" and "<body>" tags too.  Was there a reason these tags were omitted?

For status it's simple - it should have <html>.

I'm +1 with any option you choose for ExceptionHandler.


> The main reason I ask is that if optionally calling "res.resetBuffer()" in handleStatus()
is correct, I need to update HttpServletResponseFacade.sendError() to call "response.resetBuffer()"
itself to insure it gets called as required by the spec.  Otherwise, I would have handleStatus()
always call "res.resetBuffer()" as it did prior to my change.

> I am planning to modify ContextManger.java to enclose the "res.resetBuffer()" in a "try-catch"
as suggested by Danno Ferrin in an earlier e-mail.  This avoid throwing an unnecessary exception
which fouls up the intended response.  How I modify it depends on whether the resetBuffer()
call should be optional.

+1

Costin


Mime
View raw message