Bug 56581 - When an error occurs on a long JSP page, do not loose last chunk of printed text
When an error occurs on a long JSP page, do not loose last chunk of printed text
Status: RESOLVED FIXED
Product: Tomcat 8
Classification: Unclassified
Component: Catalina
8.0.8
PC All
: P2 enhancement (vote)
: ----
Assigned To: Tomcat Developers Mailing List
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2014-05-31 13:54 UTC by Konstantin Kolinko
Modified: 2014-06-14 12:31 UTC (History)
0 users



Attachments
test.jsp (265 bytes, text/plain)
2014-05-31 13:54 UTC, Konstantin Kolinko
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Konstantin Kolinko 2014-05-31 13:54:17 UTC
Created attachment 31680 [details]
test.jsp

(Reproducible with the current trunk @1598763

If there is a JSP page that generates a lot of text and then encounters an error, the client receives only 8K*n bytes of text. The last chunk of text - before the actual place of the error is lost and not sent to the client.

To reproduce:
1. Deploy the attached test.jsp and call it.
2. Actual behaviour:
The last line of text is "970 ".
If you save the result, it will be 16384 (16 Kb) (with LF line ends).

Expected behaviour:
The last line of text is "1000 Hello world!".

My motivation for this enhancement request is that seeing all the text before the error place would make it easier to locate the error.

Also the lost ~8K of text may contain something valuable for the client.

The current workaround is to look into localhost.date.log for the actual stack trace. The stacktrace is for generated java file. It may contain a JSP source snippet, but not always.
Comment 1 Konstantin Kolinko 2014-06-05 14:50:48 UTC
For reference:
Current "Error handling" thread on dev@
http://tomcat.markmail.org/thread/znillhttbmvsl5e5
Comment 2 Konstantin Kolinko 2014-06-05 14:55:11 UTC
As a note:
The error handling changes in trunk (r1600449 and follow-ups) have not fixed this issue. The behaviour is the same as earlier - the output is truncated at "970 ".

Tested with trunk @1600674.
Comment 3 Konstantin Kolinko 2014-06-06 14:06:01 UTC
Looking into generated test_jsp.java,  I see that _jspService method ends with the following:

[[[
    } catch (java.lang.Throwable t) {
      if (!(t instanceof javax.servlet.jsp.SkipPageException)){
        out = _jspx_out;
        if (out != null && out.getBufferSize() != 0)
          try { out.clearBuffer(); } catch (java.io.IOException e) {}
        if (_jspx_page_context != null) _jspx_page_context.handlePageException(t);
        else throw new ServletException(t);
      }
    } finally {
]]]

So there is a call to JspWriterImpl.clearBuffer() that clears its buffer.

I can think of two ways to fix:
a) If I had access to JspWriterImpl internals then,

if (out.flushed)
 out.flushBuffer();
else
 out.clearBuffer();

The motivation is that if we cannot undo, then do not lose what we already have generated.

I think this way is wrong. The underlying stream can have a larger buffer. While out.flushed is true, the underlying stream may not have been flushed yet.

b) 
if (response.isCommitted()) {
 out.flush();
} else {
 out.clearBuffer();
}

Although "b)" is a bit more invasive, I think it is a way to go. It is also good that it can be implemented using public APIs.
Comment 4 Konstantin Kolinko 2014-06-06 14:40:05 UTC
Committed in r1600899 and will be in 8.0.9.

By the way, the try/catch{IOException} on those lines originates from r451231.
Comment 5 Konstantin Kolinko 2014-06-14 12:31:52 UTC
Applied to Tomcat 7 in r1602584 and will be in 7.0.55 onwards.