From 6508a634fc741a92504a59ef95c24ea576f73284 Mon Sep 17 00:00:00 2001
From: Paul-Christian Volkmer <paul-christian.volkmer@mni.thm.de>
Date: Tue, 16 Oct 2012 16:46:58 +0200
Subject: [PATCH] Use exceptions instead of checking for null to send correct
http code
If a session was not found in database this will result in http 404,
if the session is not accessable for the current user the request will
result in http 403.
The old behavior was to check for null. This was the response if the
session was not found or the current user could not access this session
(not owner and inactive sessions). Both ended up with http 404 - not found.
---
.../de/thm/arsnova/SessionController.java | 14 +++++++----
.../java/de/thm/arsnova/dao/CouchDBDao.java | 6 +++--
.../exceptions/ForbiddenException.java | 5 ++++
.../arsnova/exceptions/NotFoundException.java | 5 ++++
.../controller/SessionControllerTest.java | 24 +++++++++++++++----
.../de/thm/arsnova/dao/StubDatabaseDao.java | 19 ++++++++++++++-
6 files changed, 62 insertions(+), 11 deletions(-)
create mode 100644 src/main/java/de/thm/arsnova/exceptions/ForbiddenException.java
create mode 100644 src/main/java/de/thm/arsnova/exceptions/NotFoundException.java
diff --git a/src/main/java/de/thm/arsnova/SessionController.java b/src/main/java/de/thm/arsnova/SessionController.java
index fff51bb9..fa046818 100644
--- a/src/main/java/de/thm/arsnova/SessionController.java
+++ b/src/main/java/de/thm/arsnova/SessionController.java
@@ -39,6 +39,8 @@ import org.springframework.web.bind.annotation.ResponseBody;
import de.thm.arsnova.entities.Feedback;
import de.thm.arsnova.entities.Session;
import de.thm.arsnova.entities.User;
+import de.thm.arsnova.exceptions.ForbiddenException;
+import de.thm.arsnova.exceptions.NotFoundException;
import de.thm.arsnova.services.ISessionService;
import de.thm.arsnova.services.IUserService;
import de.thm.arsnova.socket.ARSnovaSocketIOServer;
@@ -73,10 +75,14 @@ public class SessionController {
@RequestMapping(value="/session/{sessionkey}", method=RequestMethod.GET)
@ResponseBody
public Session getSession(@PathVariable String sessionkey, HttpServletResponse response) {
- Session session = sessionService.getSession(sessionkey);
- if (session != null) return session;
-
- response.setStatus(HttpStatus.NOT_FOUND.value());
+ try {
+ Session session = sessionService.getSession(sessionkey);
+ if (session != null) return session;
+ } catch (NotFoundException e) {
+ response.setStatus(HttpStatus.NOT_FOUND.value());
+ } catch (ForbiddenException e) {
+ response.setStatus(HttpStatus.FORBIDDEN.value());
+ }
return null;
}
diff --git a/src/main/java/de/thm/arsnova/dao/CouchDBDao.java b/src/main/java/de/thm/arsnova/dao/CouchDBDao.java
index e6175ad6..28ba1b8c 100644
--- a/src/main/java/de/thm/arsnova/dao/CouchDBDao.java
+++ b/src/main/java/de/thm/arsnova/dao/CouchDBDao.java
@@ -51,6 +51,8 @@ import com.fourspaces.couchdb.ViewResults;
import de.thm.arsnova.entities.Feedback;
import de.thm.arsnova.entities.Session;
import de.thm.arsnova.entities.User;
+import de.thm.arsnova.exceptions.ForbiddenException;
+import de.thm.arsnova.exceptions.NotFoundException;
import de.thm.arsnova.services.ISessionService;
import de.thm.arsnova.services.IUserService;
import de.thm.arsnova.socket.message.Question;
@@ -148,14 +150,14 @@ public class CouchDBDao implements IDatabaseDao {
public Session getSession(String keyword) {
Session result = this.getSessionFromKeyword(keyword);
if(result == null) {
- return null;
+ throw new NotFoundException();
}
if (result.isActive() || result.getCreator().equals(this.actualUserName())) {
sessionService.addUserToSessionMap(this.actualUserName(), keyword);
return result;
}
- return null;
+ throw new ForbiddenException();
}
@Override
diff --git a/src/main/java/de/thm/arsnova/exceptions/ForbiddenException.java b/src/main/java/de/thm/arsnova/exceptions/ForbiddenException.java
new file mode 100644
index 00000000..8f1f74d5
--- /dev/null
+++ b/src/main/java/de/thm/arsnova/exceptions/ForbiddenException.java
@@ -0,0 +1,5 @@
+package de.thm.arsnova.exceptions;
+
+public class ForbiddenException extends RuntimeException {
+ private static final long serialVersionUID = 1L;
+}
diff --git a/src/main/java/de/thm/arsnova/exceptions/NotFoundException.java b/src/main/java/de/thm/arsnova/exceptions/NotFoundException.java
new file mode 100644
index 00000000..caca267d
--- /dev/null
+++ b/src/main/java/de/thm/arsnova/exceptions/NotFoundException.java
@@ -0,0 +1,5 @@
+package de.thm.arsnova.exceptions;
+
+public class NotFoundException extends RuntimeException {
+ private static final long serialVersionUID = 1L;
+}
diff --git a/src/test/java/de/thm/arsnova/controller/SessionControllerTest.java b/src/test/java/de/thm/arsnova/controller/SessionControllerTest.java
index 74804a30..b1204794 100644
--- a/src/test/java/de/thm/arsnova/controller/SessionControllerTest.java
+++ b/src/test/java/de/thm/arsnova/controller/SessionControllerTest.java
@@ -1,6 +1,7 @@
package de.thm.arsnova.controller;
import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import javax.inject.Inject;
@@ -19,6 +20,7 @@ import org.springframework.web.servlet.ModelAndView;
import org.springframework.web.servlet.mvc.annotation.AnnotationMethodHandlerAdapter;
import de.thm.arsnova.SessionController;
+import de.thm.arsnova.exceptions.NotFoundException;
@RunWith(SpringJUnit4ClassRunner.class)
@ContextConfiguration(locations={
@@ -45,16 +47,30 @@ public class SessionControllerTest {
}
@Test
- public void testShouldNotGetMissingSession() {
+ public void testShouldNotGetUnknownSession() {
request.setMethod("GET");
- request.setRequestURI("/session/12345678");
+ request.setRequestURI("/session/00000000");
try {
final ModelAndView mav = handlerAdapter.handle(request, response, sessionController);
assertNull(mav);
+ assertTrue(response.getStatus() == 404);
} catch (Exception e) {
e.printStackTrace();
fail("An exception occured");
}
-
- }
+ }
+
+ @Test
+ public void testShouldNotGetForbiddenSession() {
+ request.setMethod("GET");
+ request.setRequestURI("/session/99999999");
+ try {
+ final ModelAndView mav = handlerAdapter.handle(request, response, sessionController);
+ assertNull(mav);
+ assertTrue(response.getStatus() == 403);
+ } catch (Exception e) {
+ e.printStackTrace();
+ fail("An exception occured");
+ }
+ }
}
diff --git a/src/test/java/de/thm/arsnova/dao/StubDatabaseDao.java b/src/test/java/de/thm/arsnova/dao/StubDatabaseDao.java
index 3026b3ee..a893fb60 100644
--- a/src/test/java/de/thm/arsnova/dao/StubDatabaseDao.java
+++ b/src/test/java/de/thm/arsnova/dao/StubDatabaseDao.java
@@ -4,12 +4,16 @@ import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import org.springframework.context.annotation.Scope;
import org.springframework.stereotype.Component;
import de.thm.arsnova.entities.Feedback;
import de.thm.arsnova.entities.Session;
import de.thm.arsnova.entities.User;
+import de.thm.arsnova.exceptions.ForbiddenException;
+import de.thm.arsnova.exceptions.NotFoundException;
import de.thm.arsnova.socket.message.Question;
@Component
@@ -20,6 +24,8 @@ public class StubDatabaseDao implements IDatabaseDao {
private Map<String, Feedback> stubFeedbacks = new ConcurrentHashMap<String, Feedback>();
private Map<Session, Question> stubQuestions = new ConcurrentHashMap<Session, Question>();
+ private final Logger logger = LoggerFactory.getLogger(getClass());
+
public StubDatabaseDao() {
fillWithDummySessions();
fillWithDummyFeedbacks();
@@ -56,7 +62,13 @@ public class StubDatabaseDao implements IDatabaseDao {
@Override
public Session getSession(String keyword) {
- return stubSessions.get(keyword);
+ // Magic keyword for forbidden session
+ if (keyword.equals("99999999")) throw new ForbiddenException();
+
+ Session session = stubSessions.get(keyword);
+ if (session == null) throw new NotFoundException();
+
+ return session;
}
@Override
@@ -108,4 +120,9 @@ public class StubDatabaseDao implements IDatabaseDao {
return stubQuestions.get(session) != null;
}
+ @Override
+ public Question getQuestion(String id) {
+ // Simply ... no such question ;-)
+ return null;
+ }
}
--
GitLab