killbill-memoizeit

util: fix issue with bad sessionId on H2 If a garbage JSESSIONID

7/15/2014 9:19:59 AM

Details

diff --git a/util/src/main/java/org/killbill/billing/util/security/shiro/dao/JDBCSessionDao.java b/util/src/main/java/org/killbill/billing/util/security/shiro/dao/JDBCSessionDao.java
index b068f68..c389bd7 100644
--- a/util/src/main/java/org/killbill/billing/util/security/shiro/dao/JDBCSessionDao.java
+++ b/util/src/main/java/org/killbill/billing/util/security/shiro/dao/JDBCSessionDao.java
@@ -23,6 +23,7 @@ import javax.inject.Inject;
 
 import org.apache.shiro.session.Session;
 import org.apache.shiro.session.mgt.eis.CachingSessionDAO;
+import org.killbill.commons.jdbi.mapper.LowerToCamelBeanMapperFactory;
 import org.skife.jdbi.v2.DBI;
 import org.skife.jdbi.v2.IDBI;
 import org.skife.jdbi.v2.Transaction;
@@ -30,8 +31,6 @@ import org.skife.jdbi.v2.TransactionStatus;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import org.killbill.commons.jdbi.mapper.LowerToCamelBeanMapperFactory;
-
 public class JDBCSessionDao extends CachingSessionDAO {
 
     private static final Logger log = LoggerFactory.getLogger(JDBCSessionDao.class);
@@ -72,7 +71,20 @@ public class JDBCSessionDao extends CachingSessionDAO {
 
     @Override
     protected Session doReadSession(final Serializable sessionId) {
-        final SessionModelDao sessionModelDao = jdbcSessionSqlDao.read(sessionId);
+        // Shiro should not pass us a null sessionId, but be safe...
+        if (sessionId == null) {
+            return null;
+        }
+
+        // Ignore unsupported JSESSIONID cookies
+        final Long recordId;
+        try {
+            recordId = Long.parseLong(sessionId.toString().trim());
+        } catch (final NumberFormatException e) {
+            return null;
+        }
+
+        final SessionModelDao sessionModelDao = jdbcSessionSqlDao.read(recordId);
         if (sessionModelDao == null) {
             return null;
         }
diff --git a/util/src/main/java/org/killbill/billing/util/security/shiro/dao/JDBCSessionSqlDao.java b/util/src/main/java/org/killbill/billing/util/security/shiro/dao/JDBCSessionSqlDao.java
index 5987b92..676d082 100644
--- a/util/src/main/java/org/killbill/billing/util/security/shiro/dao/JDBCSessionSqlDao.java
+++ b/util/src/main/java/org/killbill/billing/util/security/shiro/dao/JDBCSessionSqlDao.java
@@ -16,21 +16,18 @@
 
 package org.killbill.billing.util.security.shiro.dao;
 
-import java.io.Serializable;
-
+import org.killbill.commons.jdbi.binder.SmartBindBean;
 import org.skife.jdbi.v2.sqlobject.Bind;
 import org.skife.jdbi.v2.sqlobject.SqlQuery;
 import org.skife.jdbi.v2.sqlobject.SqlUpdate;
 import org.skife.jdbi.v2.sqlobject.mixins.Transactional;
 import org.skife.jdbi.v2.sqlobject.stringtemplate.UseStringTemplate3StatementLocator;
 
-import org.killbill.commons.jdbi.binder.SmartBindBean;
-
 @UseStringTemplate3StatementLocator
 public interface JDBCSessionSqlDao extends Transactional<JDBCSessionSqlDao> {
 
     @SqlQuery
-    public SessionModelDao read(@Bind("recordId") final Serializable sessionId);
+    public SessionModelDao read(@Bind("recordId") final Long sessionId);
 
     @SqlUpdate
     public void create(@SmartBindBean final SessionModelDao sessionModelDao);
diff --git a/util/src/test/java/org/killbill/billing/util/security/shiro/dao/TestJDBCSessionDao.java b/util/src/test/java/org/killbill/billing/util/security/shiro/dao/TestJDBCSessionDao.java
index 45a424f..c8da341 100644
--- a/util/src/test/java/org/killbill/billing/util/security/shiro/dao/TestJDBCSessionDao.java
+++ b/util/src/test/java/org/killbill/billing/util/security/shiro/dao/TestJDBCSessionDao.java
@@ -22,15 +22,26 @@ import java.util.UUID;
 
 import org.apache.shiro.session.Session;
 import org.apache.shiro.session.mgt.SimpleSession;
-import org.skife.jdbi.v2.DBI;
+import org.killbill.billing.util.UtilTestSuiteWithEmbeddedDB;
 import org.testng.Assert;
 import org.testng.annotations.Test;
 
-import org.killbill.billing.util.UtilTestSuiteWithEmbeddedDB;
-
 public class TestJDBCSessionDao extends UtilTestSuiteWithEmbeddedDB {
 
     @Test(groups = "slow")
+    public void testH2AndInvalidSessionId() {
+        final JDBCSessionDao jdbcSessionDao = new JDBCSessionDao(dbi);
+
+        // We need to create some data to force H2 to build the query
+        // (otherwise, the read path is optimized and the bug is not triggered)
+        final SimpleSession session = createSession();
+        jdbcSessionDao.doCreate(session);
+
+        // Make sure this doesn't throw any exception on H2
+        Assert.assertNull(jdbcSessionDao.doReadSession(UUID.randomUUID()));
+    }
+
+    @Test(groups = "slow")
     public void testCRUD() throws Exception {
         // Note! We are testing the do* methods here to bypass the caching layer
         final JDBCSessionDao jdbcSessionDao = new JDBCSessionDao(dbi);