Skip to content

Antti Väyrynen / Moya

Go to a project
Commit be84470d by Antti Tönkyrä
Options

Merge branch 'servletrace' into 'master' 

Image servlet race

There is a possible and even likely race with image servlet
which can cause random images to be returned.

Servlets can be called from multiple threads at once, but GenericImageServlet
stores request to local variable, which can be accessed from subclasses.
When called from multiple threads the request can be overriden, which can cause
random data or images to be returned.

See merge request !284
2 parents 13407a2b 5d75d07c
Inline Side-by-side
Showing with 23 additions and 23 deletions
code/moya-web/src/main/java/fi/codecrew/moya/servlet/CardTemplateServlet.java
...@@ -48,15 +48,16 @@ public class CardTemplateServlet extends GenericImageServlet { ...@@ -48,15 +48,16 @@ public class CardTemplateServlet extends GenericImageServlet {
private transient PermissionBeanLocal permbean; private transient PermissionBeanLocal permbean;
/** /**
* @see HttpServlet#doPost(HttpServletRequest request, HttpServletResponse response) * @see HttpServlet#doPost(HttpServletRequest request, HttpServletResponse
* response)
*/ */
private final Pattern lhMatcher = Pattern.compile("^127.[0-9.]*|(0:)*1%0"); private final Pattern lhMatcher = Pattern.compile("^127.[0-9.]*|(0:)*1%0");
@Override @Override
protected ImageMover getImagedata() { protected ImageMover getImagedata(HttpServletRequest request) {
ImageMover ret = new ImageMover(); ImageMover ret = new ImageMover();
CardTemplate templ = ctbean.find(getIdParam("imageid")); CardTemplate templ = ctbean.find(getIdParam("imageid", request));
if (templ != null) { // && if (templ != null) { // &&
......
code/moya-web/src/main/java/fi/codecrew/moya/servlet/FileDownloadServlet.java
...@@ -96,9 +96,9 @@ public class FileDownloadServlet extends GenericImageServlet { ...@@ -96,9 +96,9 @@ public class FileDownloadServlet extends GenericImageServlet {
* userimage/<eventuserid>.jpg cardtemplate/<cardtemplateid>.png usercard/ * userimage/<eventuserid>.jpg cardtemplate/<cardtemplateid>.png usercard/
*/ */
@Override @Override
protected ImageMover getImagedata() { protected ImageMover getImagedata(HttpServletRequest request) {
ImageMover ret = new ImageMover(); ImageMover ret = new ImageMover();
Matcher matcher = URLPATTERN.matcher(super.request.getPathInfo()); Matcher matcher = URLPATTERN.matcher(request.getPathInfo());
List<String> urlparts = new ArrayList<String>(); List<String> urlparts = new ArrayList<String>();
while (matcher.find()) { while (matcher.find()) {
...@@ -145,8 +145,7 @@ public class FileDownloadServlet extends GenericImageServlet { ...@@ -145,8 +145,7 @@ public class FileDownloadServlet extends GenericImageServlet {
int imageid = Integer.parseInt(urlparts.get(1)); int imageid = Integer.parseInt(urlparts.get(1));
UserImage image = userbean.findUserimageFORCE(imageid); UserImage image = userbean.findUserimageFORCE(imageid);
if (image != null) if (image != null) {
{
if (!permbean.isCurrentUser(image.getUser()) if (!permbean.isCurrentUser(image.getUser())
&& !permbean.hasPermission(UserPermission.VIEW_ALL)) && !permbean.hasPermission(UserPermission.VIEW_ALL))
...@@ -165,7 +164,7 @@ public class FileDownloadServlet extends GenericImageServlet { ...@@ -165,7 +164,7 @@ public class FileDownloadServlet extends GenericImageServlet {
File imagefile = null; File imagefile = null;
if (dydataRoot != null) { if (dydataRoot != null) {
imagefile = new File(dydataRoot + super.request.getPathInfo()); imagefile = new File(dydataRoot + request.getPathInfo());
} }
if (imagefile != null && !imagefile.exists()) if (imagefile != null && !imagefile.exists())
{ {
......
code/moya-web/src/main/java/fi/codecrew/moya/servlet/GenericImageServlet.java
...@@ -83,11 +83,9 @@ public abstract class GenericImageServlet extends HttpServlet { ...@@ -83,11 +83,9 @@ public abstract class GenericImageServlet extends HttpServlet {
@SuppressWarnings("unused") @SuppressWarnings("unused")
private static final Logger logger = LoggerFactory.getLogger(GenericImageServlet.class); private static final Logger logger = LoggerFactory.getLogger(GenericImageServlet.class);
protected abstract ImageMover getImagedata(); protected abstract ImageMover getImagedata(HttpServletRequest request);
protected HttpServletRequest request; protected Integer getIdParam(String name, HttpServletRequest request)
protected Integer getIdParam(String name)
{ {
Integer ret = null; Integer ret = null;
String idStr = request.getParameter(name); String idStr = request.getParameter(name);
...@@ -106,16 +104,15 @@ public abstract class GenericImageServlet extends HttpServlet { ...@@ -106,16 +104,15 @@ public abstract class GenericImageServlet extends HttpServlet {
@Override @Override
protected void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { protected void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
this.request = request;
ImageMover data = getImagedata(); ImageMover data = getImagedata(request);
if (data == null || data.getData() == null || data.getResponse() != null) { if (data == null || data.getData() == null || data.getResponse() != null) {
if (data != null && data.getData() != null) { if (data != null && data.getData() != null) {
response.setStatus(data.getResponse()); response.setStatus(data.getResponse());
} else { } else {
response.setStatus(HttpServletResponse.SC_NOT_FOUND); response.setStatus(HttpServletResponse.SC_NOT_FOUND);
} }
response.getWriter().append("Not Found"); response.getWriter().append("Error " + data.getResponse() + " while fetching data");
} else { } else {
response.setContentLength(data.getData().length); response.setContentLength(data.getData().length);
if (request.getParameter("download") != null) if (request.getParameter("download") != null)
......
code/moya-web/src/main/java/fi/codecrew/moya/servlet/UserCardPngServlet.java
...@@ -54,7 +54,8 @@ public class UserCardPngServlet extends GenericImageServlet { ...@@ -54,7 +54,8 @@ public class UserCardPngServlet extends GenericImageServlet {
private transient PermissionBeanLocal permbean; private transient PermissionBeanLocal permbean;
/** /**
* @see HttpServlet#doPost(HttpServletRequest request, HttpServletResponse response) * @see HttpServlet#doPost(HttpServletRequest request, HttpServletResponse
* response)
*/ */
// private final Pattern lhMatcher = // private final Pattern lhMatcher =
...@@ -66,12 +67,12 @@ public class UserCardPngServlet extends GenericImageServlet { ...@@ -66,12 +67,12 @@ public class UserCardPngServlet extends GenericImageServlet {
// /dydata/(userimage|...)/stuff... // /dydata/(userimage|...)/stuff...
@Override @Override
protected ImageMover getImagedata() { protected ImageMover getImagedata(HttpServletRequest request) {
ImageMover ret = new ImageMover(); ImageMover ret = new ImageMover();
List<String> urlparts = new ArrayList<String>(); List<String> urlparts = new ArrayList<String>();
if (super.request.getPathInfo() != null) if (request.getPathInfo() != null)
{ {
Matcher matcher = URLPATTERN.matcher(super.request.getPathInfo()); Matcher matcher = URLPATTERN.matcher(request.getPathInfo());
logger.info("urlparts {}", urlparts); logger.info("urlparts {}", urlparts);
while (matcher.find()) { while (matcher.find()) {
......
code/moya-web/src/main/java/fi/codecrew/moya/servlet/UserCardServlet.java
...@@ -67,7 +67,8 @@ public class UserCardServlet extends GenericImageServlet { ...@@ -67,7 +67,8 @@ public class UserCardServlet extends GenericImageServlet {
private transient PermissionBeanLocal permbean; private transient PermissionBeanLocal permbean;
/** /**
* @see HttpServlet#doPost(HttpServletRequest request, HttpServletResponse response) * @see HttpServlet#doPost(HttpServletRequest request, HttpServletResponse
* response)
*/ */
private final Pattern lhMatcher = Pattern.compile("^127.[0-9.]*|(0:)*1%0"); private final Pattern lhMatcher = Pattern.compile("^127.[0-9.]*|(0:)*1%0");
...@@ -79,9 +80,9 @@ public class UserCardServlet extends GenericImageServlet { ...@@ -79,9 +80,9 @@ public class UserCardServlet extends GenericImageServlet {
.getLogger(UserCardServlet.class); .getLogger(UserCardServlet.class);
@Override @Override
protected ImageMover getImagedata() { protected ImageMover getImagedata(HttpServletRequest request) {
ImageMover ret = new ImageMover(); ImageMover ret = new ImageMover();
Integer id = getIdParam("cardid"); Integer id = getIdParam("cardid", request);
PrintedCard card = cardbean.getCard(id); PrintedCard card = cardbean.getCard(id);
logger.info("image data id {}, card {}", id, card); logger.info("image data id {}, card {}", id, card);
......
code/moya-web/src/main/java/fi/codecrew/moya/servlet/UserPngImageServlet.java
...@@ -20,6 +20,7 @@ package fi.codecrew.moya.servlet; ...@@ -20,6 +20,7 @@ package fi.codecrew.moya.servlet;
import javax.ejb.EJB; import javax.ejb.EJB;
import javax.servlet.annotation.WebServlet; import javax.servlet.annotation.WebServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpServletResponse;
import org.slf4j.Logger; import org.slf4j.Logger;
...@@ -64,7 +65,7 @@ public class UserPngImageServlet extends GenericImageServlet { ...@@ -64,7 +65,7 @@ public class UserPngImageServlet extends GenericImageServlet {
// /dydata/(userimage|...)/stuff... // /dydata/(userimage|...)/stuff...
@Override @Override
protected ImageMover getImagedata() { protected ImageMover getImagedata(HttpServletRequest request) {
EventUser usr = permbean.getCurrentUser(); EventUser usr = permbean.getCurrentUser();
logger.info("Trying to print imagedata for user {}", usr); logger.info("Trying to print imagedata for user {}", usr);
ImageMover ret = new ImageMover(); ImageMover ret = new ImageMover();
......
Markdown is supported
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!