tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kin-Man Chung <Kin-Man.Ch...@Eng.Sun.COM>
Subject Re: costin: fix reverted
Date Tue, 05 Nov 2002 01:14:34 GMT

> X-Trace: main.gmane.org 1036447809 25208 64.84.39.162 (4 Nov 2002 22:10:09 
GMT)
> Date: Mon, 04 Nov 2002 14:11:32 -0800
> From: Costin Manolache <cmanolache@yahoo.com>
> Subject: Re: costin: fix reverted
> To: tomcat-dev@jakarta.apache.org
> X-Complaints-to: usenet@main.gmane.org
> NNTP-posting-date: Mon, 4 Nov 2002 22:10:09 +0000 (UTC)
> X-Injected-Via-Gmane: http://gmane.org/
> NNTP-posting-host: 64.84.39.162
> 
> Kin-Man Chung wrote:
> 
> > costin,
> > 
> > This fix seems to break errorPage handling in JSP, causing the errorPage
> > example to fail, and a couple of JSP watchdog tests too.  I have reverted
> > your fix.
> > 
> > I have not reverted the tomcat_4_branch.
> 
> If it breaks something - tomcat_4_branch should be the first to revert :-)
> 
I reverted tc5 for my own convenience.  I didn't revert tomcat_4_branch
becuase I thought that you may want hack more there.  :-)

> I think the main question is if releasePageContext is required by the 
> spec to flush() ( i.e. commit the message ) or not. 
> 

The spec is not specific about this.  So it is up to the implementation. :-)

> If it is required - then Content-Length just can't be set by the container
> for jsps ( which is not the end of the world :-).
> 
> If the spec doesn't require that ( and my reading is that releasePageContext 
> doc doesn't in any way imply this as a side effect - the flush() is a very
> different API ) - then I would say the tests are wrong.
> 

It's not that the tests are wrong, but that your 'fix' renders any JSP
error page not work at all.  When an exception occurs, instead of displaying
the error page, it displays the stack trace of the exception!  I don't have
anything against not calling flush() in release, but you'll also need to
make sure that it doesn't break this.

> 
> 
> Costin
> 
> 
> > 
> >> Date: Thu, 24 Oct 2002 19:18:55 +0000
> >> From: costin@apache.org
> >> Subject: cvs commit:
> > jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/runtime
> > PageContextImpl.java
> >> To: jakarta-tomcat-jasper-cvs@apache.org
> >> 
> >> costin      2002/10/24 12:18:55
> >> 
> >>   Modified:    jasper2/src/share/org/apache/jasper/runtime
> >>                         PageContextImpl.java
> >>   Log:
> >>   Change the 'flush' to just a 'flushBuffer'.
> >>   
> >>   This allows the container to deal with flushing the buffer (
> >>   wich is done automatically if the servlet doesn't explicitely
> >>   flush()/close() ). The container can attach the Content-Length
> >>   header which is usefull in many cases.
> >>   
> >>   Revision  Changes    Path
> >>   1.27      +11 -6
> > 
> 
jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/runtime/PageContextImp
> > l.java
> >>   
> >>   Index: PageContextImpl.java
> >>   ===================================================================
> >>   RCS file:
> > 
> 
/home/cvs/jakarta-tomcat-jasper/jasper2/src/share/org/apache/jasper/runtime/Page
> > ContextImpl.java,v
> >>   retrieving revision 1.26
> >>   retrieving revision 1.27
> >>   diff -u -r1.26 -r1.27
> >>   --- PageContextImpl.java   4 Oct 2002 19:21:44 -0000       1.26
> >>   +++ PageContextImpl.java   24 Oct 2002 19:18:55 -0000      1.27
> >>   @@ -162,7 +162,7 @@
> >>    this.bufferSize   = bufferSize;
> >>    this.autoFlush    = autoFlush;
> >>    this.request      = request;
> >>   -  this.response     = response;
> >>   +  this.response     = response;
> >>    
> >>    // setup session (if required)
> >>    if (request instanceof HttpServletRequest && needsSession)
> >>   @@ -209,7 +209,12 @@
> >>    ((JspWriterImpl)out).flushBuffer();
> >>    // push it into the including jspWriter
> >>    } else {
> >>   -          out.flush();
> >>   +                // Old code:
> >>   +          //out.flush();
> >>   +                // Do not flush the buffer even if we're not included
> >>   (i.e.
> >>   +                // we are the main page. The servlet will flush it and
> > close
> >>   +                // the stream.
> >>   +                ((JspWriterImpl)out).flushBuffer();
> >>    }
> >>    } catch (IOException ex) {
> >>    loghelper.log("Internal error flushing the buffer in release()");
> >>   @@ -226,7 +231,7 @@
> >>            depth = -1;
> >>    baseOut.recycle();
> >>    session      = null;
> >>   -
> >>   +
> >>    attributes.clear();
> >>        }
> >>    
> >>   
> >>   
> >>   
> >> 
> >> --
> >> To unsubscribe, e-mail:  
> >> <mailto:tomcat-dev-unsubscribe@jakarta.apache.org> For additional
> >> commands, e-mail: <mailto:tomcat-dev-help@jakarta.apache.org>
> >>
> 
> 
> 
> 
> --
> To unsubscribe, e-mail:   <mailto:tomcat-dev-unsubscribe@jakarta.apache.org>
> For additional commands, e-mail: <mailto:tomcat-dev-help@jakarta.apache.org>
> 


--
To unsubscribe, e-mail:   <mailto:tomcat-dev-unsubscribe@jakarta.apache.org>
For additional commands, e-mail: <mailto:tomcat-dev-help@jakarta.apache.org>


Mime
View raw message