Should one call .close() on HttpServletResponse.getOutputStream()/.getWriter()?
Asked Answered
V

6

102

In Java Servlets, one can access the response body via response.getOutputStream() or response.getWriter(). Should one call .close() on this OutputStream after it has been written to?

On the one hand, there is the Blochian exhortation to always close OutputStreams. On the other hand, I don't think that in this case there is an underlying resource that needs to be closed. The opening/closing of sockets is managed at the HTTP level, to allow things like persistent connections and such.

Volga answered 21/7, 2009 at 13:27 Comment(2)
You aren't invited to guess about whether there is an underlying resource to be closed. If the implementor thinks so, or rather knows so, he will provide a close() that does nothing. What you should do is close every closeable resource.Uremia
Even if your code didn't open it? I don't think so...Volga
J
100

Normally you should not close the stream. The servlet container will automatically close the stream after the servlet is finished running as part of the servlet request life-cycle.

For instance, if you closed the stream it would not be available if you implemented a Filter.

Having said all that, if you do close it nothing bad will happen as long as you don't try to use it again.

EDIT: another filter link

EDIT2: adrian.tarau is correct in that if you want to alter the response after the servlet has done its thing you should create a wrapper extending HttpServletResponseWrapper and buffer the output. This is to keep the output from going directly to the client but also allows you to protect if the servlet closes the stream, as per this excerpt (emphasis mine):

A filter that modifies a response must usually capture the response before it is returned to the client. The way to do this is to pass the servlet that generates the response a stand-in stream. The stand-in stream prevents the servlet from closing the original response stream when it completes and allows the filter to modify the servlet's response.

Article

One can infer from that official Sun article that closing the OutputStream from a servlet is something that is a normal occurrence, but is not mandatory.

Jenevajeni answered 21/7, 2009 at 13:38 Comment(3)
This is correct. One thing to note is that in some cases you may need to flush the stream, and that is perfectly permissible.Yvette
There is another side effect of closing the writer. You will also not be able to set the status code through response.setStatus after it has been closed.Idelson
Follow this advice. It will save you a lot of pain. I wouldn't flush() either unless you know why you're doing it -- you should let the container handle buffering.Polyandry
A
87

The general rule of them is this: if you opened the stream, then you should close it. If you didn't, you shouldn't. Make sure the code is symmetric.

In the case of HttpServletResponse, it's a bit less clear cut, since it's not obvious if calling getOutputStream() is an operation that opens the stream. The Javadoc just says that it "Returns a ServletOutputStream"; similarly for getWriter(). Either way, what is clear is that HttpServletResponse "owns" the stream/writer, and it (or the container) is responsible for closing it again.

So to answer your question - no, you should not close the stream in this case. The container must do that, and if you get in there before it, you risk introducing subtle bugs in your application.

Aric answered 21/7, 2009 at 15:18 Comment(7)
I agree with this answer, you may also want to checkout the ServletResponse.flushBuffer() see: docs.oracle.com/javaee/1.4/api/javax/servlet/…Fob
"if you opened the stream, then you should close it. If you didn't, you shouldn't" --- well saidJoell
It feels like the sentences posted on the school board "if you open it, close it. If you turn it on, turn it off. If you unlock it, lock it up. [...]"Blockus
Iv noticed that is i use the stream early in my code and never again, the client waits for the whole servlet to execute, but when i call close() when im done with the stream, the client returns immediately and the rest of the servlet continues executing. Doesnt that then make the answer a bit more relative? as opposed to a definitive yes or noBronwynbronx
"if you opened the stream, then you should close it. If you didn't, you shouldn't. Make sure the code is symmetric." - So what if you then create another stream that wraps this stream? Hard to stay symmetric in this case since calling close out the outer stream will typically close the nested one.Pollinize
If a component API accepts a stream for writing some kind of document, it is not a good idea for this API to close the stream? For example something that produces a PDF document, if it's closing the stream I would consider this to be incorrect of the API. I'm trying to remember where I might have had this issue, maybe with iText that when not calling close on the stream the client would receive an exception.Arbogast
As an example on my prior comment, looking at FasterXML's Jackson ObjectMapper (source), without digging too deeply it appears that writeValue(DataOutput, Object) calls _configAndWriteValue(JsonGenerator, Object) which does close the stream.Arbogast
N
5

If there is any chance the filter might be called on an 'included' resource, you should definitely not close the stream. This will cause the including resource to fail with a 'stream closed' exception.

Newborn answered 15/10, 2010 at 17:58 Comment(1)
Thanks for adding this comment. Hilariously enough I am still grappling with the problem a little bit -- just now I noticed NetBeans's servlet template does include code to close the output stream...Volga
P
5

Another argument against closing the OutputStream. Look at this servlet. It throws an exception. The exception is mapped in the web.xml to an error JSP:

package ser;

import java.io.*;
import javax.servlet.ServletException;
import javax.servlet.annotation.WebServlet;
import javax.servlet.http.*;

@WebServlet(name = "Erroneous", urlPatterns = {"/Erroneous"})
public class Erroneous extends HttpServlet {

  protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
    resp.setContentType("text/html;charset=UTF-8");
    PrintWriter out = resp.getWriter();
    try {
      throw new IOException("An error");
    } finally {
//      out.close();
    }
  }
}

The web.xml file contains:

<?xml version="1.0" encoding="UTF-8"?>
<web-app version="3.0" xmlns="http://java.sun.com/xml/ns/javaee" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://java.sun.com/xml/ns/javaee http://java.sun.com/xml/ns/javaee/web-app_3_0.xsd">
    <session-config>
        <session-timeout>
            30
        </session-timeout>
    </session-config>
    <error-page>
        <exception-type>java.io.IOException</exception-type>
        <location>/error.jsp</location>
    </error-page>
</web-app>

And the error.jsp:

<%@page contentType="text/html" pageEncoding="UTF-8" isErrorPage="true"%>
<!DOCTYPE html>
<html>
    <head>
        <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
        <title>Error Page</title>
    </head>
    <body>
        <h1><%= exception.getMessage()%></h1>
    </body>
</html>

When you load /Erroneous in the browser you see the error page displaying "An error". But if you un-comment the out.close() line in the above servlet, redeploy de application, and reload /Erroneous you’ll see nothing in the browser. I have no clue about what is actually happening, but I guess that out.close() prevents the error handling.

Tested with Tomcat 7.0.50, Java EE 6 using Netbeans 7.4.

Putput answered 19/2, 2014 at 15:42 Comment(0)
W
4

You should close the stream, the code is cleaner since you invoke getOutputStream() and the stream is not passed to you as a parameter, when usually you just use it and don't attempt to close it. The Servlet API doesn't states that if the output stream can be closed or must not be closed, in this case you can safely close the stream, any container out there takes care of closing the stream if it was not closed by the servlet.

Here is the close() method in Jetty, they close the stream if it not closed.

public void close() throws IOException
    {
        if (_closed)
            return;

        if (!isIncluding() && !_generator.isCommitted())
            commitResponse(HttpGenerator.LAST);
        else
            flushResponse();

        super.close();
    }

Also as a developer of a Filter you should not presume that the OutputStream is not closed, you should always pass another OutputStream if you want to alter the content after the servlet has done its job.

EDIT : I'm always closing the stream and I didn't had any problems with Tomcat/Jetty. I don't think you should have any problems with any container, old or new.

Wain answered 21/7, 2009 at 15:8 Comment(4)
"You should close the stream, the code is cleaner..." - To me, code peppered with .close() looks less clean than code without, particularly if the .close() is unnecessary -- which is what this question is attempting to determine.Volga
yep, but beauty comes after :) Anyway, since the API is not clear I would prefer to close it, the code looks consistent, once you request an OutputStream you should close it unless the API says "don't close it".Wain
I don't think closing the output stream in my own code is a good practice. That's the job of the container.Coated
get* does not create the stream, it is not it's producer. You should not close it, the container must do it.Pryer
B
1

If you're using Spring with Spring Security, you should not close the stream or writer.

Stream returned from ServletResponse.getOutputStream() or writer returned from ServletResponse.getWriter() will commit the response upon closing. Committing the response, as explained here, means that http status and headers become immutable and Spring framework will not be able to adjust http status even if exception gets thrown during serving this request.

An instance of OnCommittedResponseWrapper class is used as implementation of ServletResponse and here's the code responsible for this behavior (check javadoc as well).

Consider the following example controller:

@RestController
public class MyController {
    @RequestMapping(method = RequestMethod.POST, value = "/blah")
    public void entrypoint(ServletRequest request, ServletResponse response) throws IOException {
        try (var writer = response.getWriter()) {
            throw new RuntimeException("Something bad happened here");
        }
    }

When the exception is thrown, the first thing that happens is a call to writer.close() that'll freeze response http status to its default value 200.

Only after that the exception will start propagating from this controller to Spring error handlers. Spring error handlers will not be able to change the status to 500 because the response was committed already and so the status will remain 200.

Borden answered 7/12, 2020 at 16:4 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.