Bug 46051

Summary: Servlet response writer does not respect line.separator system property
Product: Tomcat 6 Reporter: Chris Hubick <chris>
Component: CatalinaAssignee: Tomcat Developers Mailing List <dev>
Status: RESOLVED FIXED    
Severity: normal    
Priority: P2    
Version: unspecified   
Target Milestone: default   
Hardware: All   
OS: All   
Attachments: remove special CoyoteWriter println handling

Description Chris Hubick 2008-10-21 10:54:42 UTC 
Created attachment 22763 [details]
remove special CoyoteWriter println handling

public void doGet(final HttpServletRequest request, final HttpServletResponse response) throws ServletException, IOException {
  response.setContentType("text/plain");
  PrintWriter writer = response.getWriter();
  writer.print("hello world");
  writer.println(); //FIXME ignores System.getProperty("line.separator") and always outputs \r\n
  return;
}

This means that if you use a servlet Writer on a Unix system to output any text based data format which is defined as requiring Unix format line endings ('\n'), that data will be corrupted.

This appears to be because org.apache.catalina.connector.CoyoteWriter does:
--------
private static final char[] LINE_SEP = { '\r', '\n' };

public void println() {
  write(LINE_SEP);
}
--------

This used to work in Tomcat 4.

Looking at the old code at http://svn.apache.org/repos/asf/tomcat/archive/tc4.0.x/tags/tc4.0.6/connectors/coyote/src/java/org/apache/coyote/tomcat4/CoyoteWriter.java it appears to defer line endings to the PrintWriter base class which does the right thing.

Was this change was made to output HTTP headers properly or something?  It specifically seems to override the default behavior to do this.  I don't know the code well enough to determine if the attached patch to revert to the default behavior again would break something?

Thanks for your consideration.
Comment 1 Mark Thomas 2008-10-21 12:23:58 UTC 
The change is http://svn.apache.org/viewvc?view=rev&revision=298149

I am tempted to leave this as is. Whilst strict adherence to the PrintWriter interface requires that System.getProperty("line.separator") is used, any truly portable web application is going to have to write it's own new lines anyway to be resilient against the case where it runs on a different platform.

If this was fixed, I would change the initialisation of LINE_SEP.
Comment 2 Chris Hubick 2008-10-21 12:50:48 UTC 
I see where you are coming from, and somewhat agree - though I think there is value in strict adherance to the API.  The decision is yours - I just wanted to make sure you were aware of it.

Luckily, I had the code to this servlet and patched it to work around this.

Just to note, we were actually bit by this bug when we upgraded from Tomcat 4 on our CAS ( http://www.ja-sig.org/products/cas/ ) central authentication server.  The Yale CAS server code writes out XML using the response Writer, and after the upgrade this output then contained the additional '\r'.  Of course, this technically shouldn't matter to the XML either, except that the old (2.0.10) mod_cas C client we are using on some old servers apparently doesn't use a real XML parser and breaks reading these line endings in.  We can't upgrade the client in our legacy environment because their newer mod_auth_cas client requires a newer Apache.  So, yeah, I patched the server to work as before, and we are working to upgrade all our old client servers, but as with any enterprise, that takes time.
Comment 3 Mark Thomas 2008-10-22 12:59:36 UTC 
Just testing a new way to provide an svn link:
r298149
or
revision 298149
Comment 4 Mark Thomas 2008-10-23 12:43:13 UTC 
Testing svn links in mail messages - please ignore.

r298149
or
revision 298149
Comment 5 Mark Thomas 2008-10-23 12:53:23 UTC 
Testing svn links in mail messages again - please ignore.

r298149
or
revision 298149
Comment 6 Mark Thomas 2008-11-01 17:49:30 UTC 
I've fixed this in trunk. I am not going to propose it for back-porting since it might break something and it doesn't make much sense for app servers anyway. But since there is a spec it will be in 7.x
Comment 7 Chris Hubick 2008-11-01 19:31:46 UTC 
Sounds good, thanks! :)