Servlets should not use mutable fields without synchronization JAVA-E0128
logger is accessed without synchronization
131
132 public static final boolean isMultipartRequest(HttpServletRequest request) {
133 // inspired by org.apache.commons.fileupload
134 logger.debug("content-type " + request.getContentType());135 return REQUEST_METHOD_POST.equalsIgnoreCase(request.getMethod())
136 && request.getContentType() != null
137 && request.getContentType().toLowerCase()logger is accessed without synchronization
106 throw new ServletException(
107 "Error while transcoding svg file to an image");
108 } catch (TranscoderException te){
109 logger.error("Error while transcoding svg file to an image", te);110 stream.close();
111 throw new ServletException(
112 "Error while transcoding svg file to an image");logger is accessed without synchronization
101 stream = SVGRasterizer.getInstance().transcode(stream, svg,
102 mime, width);
103 } catch (SVGRasterizerException sre) {
104 logger.error("Error while transcoding svg file to an image", sre);105 stream.close();
106 throw new ServletException(
107 "Error while transcoding svg file to an image");logger is accessed without synchronization
79 + ioe.getMessage());
80 sendError(request, response, ioe);
81 } catch ( ServletException sce) {
82 logger.error("Oops something happened here redirect to error-page, " 83 + sce.getMessage());
84 sendError(request, response, sce);
85 }logger is accessed without synchronization
75 width, mime, response);
76
77 } catch (IOException ioe) {
78 logger.error("Oops something happened here redirect to error-page, " 79 + ioe.getMessage());
80 sendError(request, response, ioe);
81 } catch ( ServletException sce) {
Description
A web server generally only creates one instance of servlet or JSP class (i.e., treats the class as a Singleton), and will have multiple threads invoke methods on that instance to service multiple simultaneous requests.
Bad Practice
class MyServlet extends HttpServlet {
private HashMap<String, User> users; // This field may be left open to concurrent modification.
// ...
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
resp.setStatus(200);
resp.setHeader("Content-Type", "application/json");
String name = req.getParameter("name");
users.put(name, ...); // This access is not synchronized and could result in concurrent modification of users.
}
}
Accessing such variables without synchronizing on them could allow ConcurrentModificationExceptions. This could also result in race conditions occurring between threads that modify the concerned field.
Recommended
Consider using some form of synchronization to ensure that such variables can be accessed safely in a concurrent context.
private synchronized doOperationOnUsers(String name) {
// users is only modified within this method.
users.put(name, ...);
}
References
- SpotBugs - MSF_MUTABLE_SERVLET_FIELD
- CWE-362 - Concurrent Execution using Shared Resource with Improper Synchronization ('Race Condition')