From 7b218e2ffa59fb830234ae3d3604d39d4ad0ad36 Mon Sep 17 00:00:00 2001 From: Cuba Date: Fri, 4 Dec 2020 09:34:16 +0000 Subject: [PATCH] Revert "Merge pull request #4479 from sgflt/issue-4280-improper-session-synchronization" This reverts commit 0892dff883e46bf5b1203aed626e361c3ffe2a24, reversing changes made to 39d14da22e989187450419662b57c2298b008aad. --- .../java/org/apache/catalina/Manager.java | 110 +++++++++--------- .../catalina/authenticator/SingleSignOn.java | 33 +++--- .../session/CookiePersistentManager.java | 5 +- .../apache/catalina/session/ManagerBase.java | 16 ++- .../session/PersistentManagerBase.java | 81 ++++++++----- .../catalina/session/StandardManager.java | 57 ++++----- .../catalina/session/StandardSession.java | 4 +- .../security/web/GlassFishSingleSignOn.java | 70 ++++++----- .../web/ha/authenticator/HASingleSignOn.java | 56 ++++----- 9 files changed, 242 insertions(+), 190 deletions(-) diff --git a/appserver/web/web-core/src/main/java/org/apache/catalina/Manager.java b/appserver/web/web-core/src/main/java/org/apache/catalina/Manager.java index 6604f1939bf..04225340338 100644 --- a/appserver/web/web-core/src/main/java/org/apache/catalina/Manager.java +++ b/appserver/web/web-core/src/main/java/org/apache/catalina/Manager.java @@ -65,7 +65,6 @@ import javax.servlet.http.*; import java.beans.PropertyChangeListener; import java.io.IOException; -import java.util.List; //END OF 6364900 /** @@ -95,20 +94,20 @@ public interface Manager { /** * Return the Container with which this Manager is associated. */ - Container getContainer(); + public Container getContainer(); /** * Set the Container with which this Manager is associated. * * @param container The newly associated Container */ - void setContainer(Container container); + public void setContainer(Container container); /** * Return the distributable flag for the sessions supported by * this Manager. */ - boolean getDistributable(); + public boolean getDistributable(); /** * Set the distributable flag for the sessions supported by this @@ -117,30 +116,30 @@ public interface Manager { * * @param distributable The new distributable flag */ - void setDistributable(boolean distributable); + public void setDistributable(boolean distributable); /** * Return descriptive information about this Manager implementation and * the corresponding version number, in the format * <description>/<version>. */ - String getInfo(); + public String getInfo(); /** * Same as getMaxInactiveIntervalSeconds */ - int getMaxInactiveInterval(); + public int getMaxInactiveInterval(); /** * Return the default maximum inactive interval (in seconds) * for Sessions created by this Manager. */ - int getMaxInactiveIntervalSeconds(); + public int getMaxInactiveIntervalSeconds(); /** * Same as setMaxInactiveIntervalSeconds */ - void setMaxInactiveInterval(int interval); + public void setMaxInactiveInterval(int interval); /** * Set the default maximum inactive interval (in seconds) @@ -148,7 +147,7 @@ public interface Manager { * * @param interval The new default value */ - void setMaxInactiveIntervalSeconds(int interval); + public void setMaxInactiveIntervalSeconds(int interval); /** * Gets the session id length (in bytes) of Sessions created by @@ -156,7 +155,7 @@ public interface Manager { * * @return The session id length */ - int getSessionIdLength(); + public int getSessionIdLength(); /** * Sets the session id length (in bytes) for Sessions created by this @@ -164,31 +163,31 @@ public interface Manager { * * @param length The session id length */ - void setSessionIdLength(int length); + public void setSessionIdLength(int length); /** * Same as getSessionCount */ - int getSessionCounter(); + public int getSessionCounter(); /** * Returns the total number of sessions created by this manager. * * @return Total number of sessions created by this manager. */ - int getSessionCount(); + public int getSessionCount(); /** * Same as setSessionCount */ - void setSessionCounter(int sessionCounter); + public void setSessionCounter(int sessionCounter); /** * Sets the total number of sessions created by this manager. * * @param sessionCounter Total number of sessions created by this manager. */ - void setSessionCount(int sessionCounter); + public void setSessionCount(int sessionCounter); /** * Gets the maximum number of sessions that have been active at the same @@ -197,7 +196,7 @@ public interface Manager { * @return Maximum number of sessions that have been active at the same * time */ - int getMaxActive(); + public int getMaxActive(); /** * (Re)sets the maximum number of sessions that have been active at the @@ -206,28 +205,28 @@ public interface Manager { * @param maxActive Maximum number of sessions that have been active at * the same time. */ - void setMaxActive(int maxActive); + public void setMaxActive(int maxActive); /** * Gets the number of currently active sessions. * * @return Number of currently active sessions */ - int getActiveSessions(); + public int getActiveSessions(); /** * Gets the number of sessions that have expired. * * @return Number of sessions that have expired */ - int getExpiredSessions(); + public int getExpiredSessions(); /** * Sets the number of sessions that have expired. * * @param expiredSessions Number of sessions that have expired */ - void setExpiredSessions(int expiredSessions); + public void setExpiredSessions(int expiredSessions); /** * Gets the number of sessions that were not created because the maximum @@ -235,7 +234,7 @@ public interface Manager { * * @return Number of rejected sessions */ - int getRejectedSessions(); + public int getRejectedSessions(); /** * Sets the number of sessions that were not created because the maximum @@ -243,12 +242,12 @@ public interface Manager { * * @param rejectedSessions Number of rejected sessions */ - void setRejectedSessions(int rejectedSessions); + public void setRejectedSessions(int rejectedSessions); /** * Same as getSessionMaxAliveTimeSeconds */ - int getSessionMaxAliveTime(); + public int getSessionMaxAliveTime(); /** * Gets the longest time (in seconds) that an expired session had been @@ -257,12 +256,12 @@ public interface Manager { * @return Longest time (in seconds) that an expired session had been * alive. */ - int getSessionMaxAliveTimeSeconds(); + public int getSessionMaxAliveTimeSeconds(); /** * Same as setSessionMaxAliveTimeSeconds */ - void setSessionMaxAliveTime(int sessionMaxAliveTime); + public void setSessionMaxAliveTime(int sessionMaxAliveTime); /** * Sets the longest time (in seconds) that an expired session had been @@ -271,12 +270,12 @@ public interface Manager { * @param sessionMaxAliveTime Longest time (in seconds) that an expired * session had been alive. */ - void setSessionMaxAliveTimeSeconds(int sessionMaxAliveTime); + public void setSessionMaxAliveTimeSeconds(int sessionMaxAliveTime); /** * Same as getSessionAverageAliveTimeSeconds */ - int getSessionAverageAliveTime(); + public int getSessionAverageAliveTime(); /** * Gets the average time (in seconds) that expired sessions had been @@ -285,12 +284,12 @@ public interface Manager { * @return Average time (in seconds) that expired sessions had been * alive. */ - int getSessionAverageAliveTimeSeconds(); + public int getSessionAverageAliveTimeSeconds(); /** * Same as setSessionAverageAliveTimeSeconds */ - void setSessionAverageAliveTime(int sessionAverageAliveTime); + public void setSessionAverageAliveTime(int sessionAverageAliveTime); /** * Sets the average time (in seconds) that expired sessions had been @@ -299,7 +298,7 @@ public interface Manager { * @param sessionAverageAliveTime Average time (in seconds) that expired * sessions had been alive. */ - void setSessionAverageAliveTimeSeconds(int sessionAverageAliveTime); + public void setSessionAverageAliveTimeSeconds(int sessionAverageAliveTime); // --------------------------------------------------------- Public Methods @@ -309,14 +308,14 @@ public interface Manager { * * @param session Session to be added */ - void add(Session session); + public void add(Session session); /** * Add a property change listener to this component. * * @param listener The listener to add */ - void addPropertyChangeListener(PropertyChangeListener listener); + public void addPropertyChangeListener(PropertyChangeListener listener); /** * Change the session ID of the current session to a new randomly generated @@ -324,14 +323,14 @@ public interface Manager { * * @param session The session to change the session ID for */ - void changeSessionId(Session session); + public void changeSessionId(Session session); /** * Get a session from the recycled ones or create a new empty one. * The PersistentManager manager does not need to create session data * because it reads it from the Store. - */ - Session createEmptySession(); + */ + public Session createEmptySession(); /** * Construct and return a new session object, based on the default @@ -343,7 +342,7 @@ public interface Manager { * @exception IllegalStateException if a new session cannot be * instantiated for any reason */ - Session createSession(); + public Session createSession(); // START S1AS8PE 4817642 /** @@ -359,7 +358,7 @@ public interface Manager { * @return the new session, or null if a session with the * requested id already exists */ - Session createSession(String sessionId); + public Session createSession(String sessionId); // END S1AS8PE 4817642 /** @@ -373,7 +372,7 @@ public interface Manager { * @exception IOException if an input/output error occurs while * processing this request */ - Session findSession(String id) throws IOException; + public Session findSession(String id) throws IOException; /** * Finds and returns the session with the given id that also satisfies @@ -393,7 +392,7 @@ public interface Manager { * * @exception IOException if an IO error occurred */ - Session findSession(String id, String version) throws IOException; + public Session findSession(String id, String version) throws IOException; /** * Gets the session with the given id from the given request. @@ -403,7 +402,7 @@ public interface Manager { * @return the requested session, or null if not found * @throws IOException */ - Session findSession(String id, HttpServletRequest request) throws IOException; + public Session findSession(String id, HttpServletRequest request) throws IOException; /** * Returns true if this session manager supports session versioning, false @@ -412,14 +411,13 @@ public interface Manager { * @return true if this session manager supports session versioning, false * otherwise. */ - boolean isSessionVersioningSupported(); + public boolean isSessionVersioningSupported(); /** * Return the set of active Sessions associated with this Manager. - * If this Manager has no active Sessions, a empty list is returned. - * @return associated sessions + * If this Manager has no active Sessions, a zero-length array is returned. */ - List findSessions(); + public Session[] findSessions(); /** * Load any currently active sessions that were previously unloaded @@ -430,21 +428,21 @@ public interface Manager { * found during the reload * @exception IOException if an input/output error occurs */ - void load() throws ClassNotFoundException, IOException; + public void load() throws ClassNotFoundException, IOException; /** * Remove this Session from the active Sessions for this Manager. * * @param session Session to be removed */ - void remove(Session session); + public void remove(Session session); /** * Remove a property change listener from this component. * * @param listener The listener to remove */ - void removePropertyChangeListener(PropertyChangeListener listener); + public void removePropertyChangeListener(PropertyChangeListener listener); /** * Save any currently active sessions in the appropriate persistence @@ -453,21 +451,21 @@ public interface Manager { * * @exception IOException if an input/output error occurs */ - void unload() throws IOException; + public void unload() throws IOException; //PWC Extension //START OF RIMOD# 4820359 -- Support for iWS6.0 session managers /** * Perform any operations when the request is finished. */ - void update(HttpSession session) throws Exception; + public void update(HttpSession session) throws Exception; //END OF RIMOD# 4820359 //START OF 6364900 - boolean lockSession(ServletRequest request) throws ServletException; - void unlockSession(ServletRequest request); - void preRequestDispatcherProcess(ServletRequest request, ServletResponse response); - void postRequestDispatcherProcess(ServletRequest request, ServletResponse response); + public boolean lockSession(ServletRequest request) throws ServletException; + public void unlockSession(ServletRequest request); + public void preRequestDispatcherProcess(ServletRequest request, ServletResponse response); + public void postRequestDispatcherProcess(ServletRequest request, ServletResponse response); //END OF 6364900 /** @@ -477,7 +475,7 @@ public interface Manager { * @return the cookie representation of the given session * @throws IOException */ - Cookie toCookie(Session session) throws IOException; + public Cookie toCookie(Session session) throws IOException; /** * Checks the given session attribute name and value to make sure they comply with any @@ -491,5 +489,5 @@ public interface Manager { * @throws IllegalArgumentException if the given session attribute name or value violate * any restrictions set forth by this session manager */ - void checkSessionAttribute(String name, Object value) throws IllegalArgumentException; + public void checkSessionAttribute(String name, Object value) throws IllegalArgumentException; } diff --git a/appserver/web/web-core/src/main/java/org/apache/catalina/authenticator/SingleSignOn.java b/appserver/web/web-core/src/main/java/org/apache/catalina/authenticator/SingleSignOn.java index 5d32aebef13..59396c4e68e 100644 --- a/appserver/web/web-core/src/main/java/org/apache/catalina/authenticator/SingleSignOn.java +++ b/appserver/web/web-core/src/main/java/org/apache/catalina/authenticator/SingleSignOn.java @@ -57,11 +57,12 @@ */ // Portions Copyright [2016-2019] [Payara Foundation and/or its affiliates] package org.apache.catalina.authenticator; + import java.io.IOException; import java.security.Principal; import java.text.MessageFormat; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; +import java.util.HashMap; +import java.util.Map; import java.util.logging.Level; import javax.servlet.ServletException; @@ -118,7 +119,7 @@ public class SingleSignOn extends ValveBase * The cache of SingleSignOnEntry instances for authenticated Principals, keyed by the cookie value that is used to * select them. */ - protected final ConcurrentMap cache = new ConcurrentHashMap<>(); + protected Map cache = new HashMap(); /** * The lifecycle event support for this component. @@ -433,7 +434,9 @@ protected void deregister(String ssoId, Session session) { // see if we are the last session, if so blow away ssoId if (sso.isEmpty()) { - this.cache.remove(ssoId); + synchronized (cache) { + cache.remove(ssoId); + } } } @@ -446,21 +449,17 @@ protected void deregister(String ssoId, Session session) { * @param username Username used to authenticate this user * @param password Password used to authenticate this user */ - protected void register( - final String ssoId, - final Principal principal, - final String authType, - final String username, - final char[] password, - final String realmName - ) { + protected void register(String ssoId, Principal principal, String authType, String username, char[] password, String realmName) { + if (debug >= 1) { String msg = MessageFormat.format(rb.getString(LogFacade.REGISTERING_SSO_INFO), new Object[] { ssoId, principal.getName(), authType }); log(msg); } + synchronized (cache) { + cache.put(ssoId, new SingleSignOnEntry(ssoId, 0L, principal, authType, username, realmName)); + } - this.cache.put(ssoId, new SingleSignOnEntry(ssoId, 0L, principal, authType, username, realmName)); } // ------------------------------------------------------ Protected Methods @@ -502,8 +501,12 @@ protected void log(String message, Throwable t) { * * @param ssoId Single sign on identifier to look up */ - protected SingleSignOnEntry lookup(final String ssoId) { - return this.cache.get(ssoId); + protected SingleSignOnEntry lookup(String ssoId) { + + synchronized (cache) { + return cache.get(ssoId); + } + } /** diff --git a/appserver/web/web-core/src/main/java/org/apache/catalina/session/CookiePersistentManager.java b/appserver/web/web-core/src/main/java/org/apache/catalina/session/CookiePersistentManager.java index a992bb82be1..73b29931e90 100644 --- a/appserver/web/web-core/src/main/java/org/apache/catalina/session/CookiePersistentManager.java +++ b/appserver/web/web-core/src/main/java/org/apache/catalina/session/CookiePersistentManager.java @@ -95,6 +95,7 @@ public Session findSession(String id, HttpServletRequest request) throws IOExcep if (cookies == null) { return null; } + String value = null; for (Cookie cookie : cookies) { if (cookieName.equals(cookie.getName())) { return parseSession(cookie.getValue(), request.getRequestedSessionId()); @@ -111,8 +112,8 @@ public void clearSessions() { } @Override - public List findSessions() { - return Collections.emptyList(); + public Session[] findSessions() { + return null; } @Override diff --git a/appserver/web/web-core/src/main/java/org/apache/catalina/session/ManagerBase.java b/appserver/web/web-core/src/main/java/org/apache/catalina/session/ManagerBase.java index 3b7d371abef..1f87ef8fcf0 100644 --- a/appserver/web/web-core/src/main/java/org/apache/catalina/session/ManagerBase.java +++ b/appserver/web/web-core/src/main/java/org/apache/catalina/session/ManagerBase.java @@ -219,7 +219,7 @@ public abstract class ManagerBase implements Manager { * The set of currently active Sessions for this Manager, keyed by * session identifier. */ - protected final Map sessions = new ConcurrentHashMap<>(); + protected Map sessions = new ConcurrentHashMap(); // Number of sessions created by this manager protected int sessionCounter=0; @@ -896,11 +896,19 @@ public void clearSessions() { } + /** + * Return the set of active Sessions associated with this Manager. + * If this Manager has no active Sessions, a zero-length array is returned. + */ @Override - public List findSessions() { + public Session[] findSessions() { // take a snapshot - Collection sessionsValues = this.sessions.values(); - return new ArrayList<>(sessionsValues); + Collection sessionsValues = sessions.values(); + List list = new ArrayList(sessionsValues.size()); + for (Session session : sessionsValues) { + list.add(session); + } + return list.toArray(new Session[list.size()]); } diff --git a/appserver/web/web-core/src/main/java/org/apache/catalina/session/PersistentManagerBase.java b/appserver/web/web-core/src/main/java/org/apache/catalina/session/PersistentManagerBase.java index 1bb11fbff48..8836ef6308b 100644 --- a/appserver/web/web-core/src/main/java/org/apache/catalina/session/PersistentManagerBase.java +++ b/appserver/web/web-core/src/main/java/org/apache/catalina/session/PersistentManagerBase.java @@ -594,14 +594,21 @@ public void clearStore() { * Hercules: modified method */ protected void processExpires() { + if (!started) return; - final List sessions = findSessions(); - for (final Session session1 : sessions) { + Session sessions[] = findSessions(); + + for (Session session1 : sessions) { StandardSession session = (StandardSession) session1; + /* START CR 6363689 + if (!session.isValid()) { + */ + // START CR 6363689 if(!session.getIsValid() || session.hasExpired()) { - if(session.lockBackground()) { + // END CR 6363689 + if(session.lockBackground()) { try { session.expire(); } finally { @@ -944,21 +951,20 @@ public void unload() { if (store == null) return; - final List sessions = findSessions(); - if (sessions.isEmpty()) + Session sessions[] = findSessions(); + int n = sessions.length; + if (n == 0) return; if (log.isLoggable(Level.FINE)) { - log.log(Level.FINE, LogFacade.SAVING_PERSISTED_SESSION, sessions.size()); + log.log(Level.FINE, LogFacade.SAVING_PERSISTED_SESSION, String.valueOf(n)); } - - for (final Session session : sessions) { + for (int i = 0; i < n; i++) try { - swapOut(session); + swapOut(sessions[i]); } catch (IOException e) { // This is logged in writeSession() } - } } @@ -1255,12 +1261,11 @@ public void stop() throws LifecycleException { unload(); } else { // Expire all active sessions - final List sessions = findSessions(); - for (final Session session1 : sessions) { - final StandardSession session = (StandardSession) session1; - if (!session.isValid()) { + Session sessions[] = findSessions(); + for (Session session1 : sessions) { + StandardSession session = (StandardSession) session1; + if (!session.isValid()) continue; - } session.expire(); } } @@ -1315,15 +1320,15 @@ protected void processMaxIdleSwaps() { if (!isStarted() || maxIdleSwap < 0) return; - final List sessions = findSessions(); - final long timeNow = System.currentTimeMillis(); + Session sessions[] = findSessions(); + long timeNow = System.currentTimeMillis(); // Swap out all sessions idle longer than maxIdleSwap // FIXME: What's preventing us from mangling a session during // a request? if (maxIdleSwap >= 0) { - for (final Session session1 : sessions) { - final StandardSession session = (StandardSession) session1; + for (Session session1 : sessions) { + StandardSession session = (StandardSession) session1; if (!session.isValid()) continue; int timeIdle = // Truncate, do not round up @@ -1349,26 +1354,27 @@ protected void processMaxIdleSwaps() { * Hercules: modified method */ protected void processMaxActiveSwaps() { + if (!isStarted() || getMaxActiveSessions() < 0) return; - final List sessions = findSessions(); + Session sessions[] = findSessions(); // FIXME: Smarter algorithm (LRU) - if (sessions.size() <= getMaxActiveSessions()) + if (getMaxActiveSessions() >= sessions.length) return; if(log.isLoggable(Level.FINE)) { - log.log(Level.FINE, LogFacade.TOO_MANY_ACTIVE_SESSION, sessions.size()); + log.log(Level.FINE, LogFacade.TOO_MANY_ACTIVE_SESSION, sessions.length); } - int toswap = sessions.size() - getMaxActiveSessions(); + int toswap = sessions.length - getMaxActiveSessions(); long timeNow = System.currentTimeMillis(); - for (int i = 0; i < sessions.size() && toswap > 0; i++) { + for (int i = 0; i < sessions.length && toswap > 0; i++) { int timeIdle = // Truncate, do not round up - (int) ((timeNow - sessions.get(i).getLastAccessedTime()) / 1000L); + (int) ((timeNow - sessions[i].getLastAccessedTime()) / 1000L); if (timeIdle > minIdleSwap) { - StandardSession session = (StandardSession) sessions.get(i); + StandardSession session = (StandardSession) sessions[i]; //skip the session if it cannot be locked if(session.lockBackground()) { if(log.isLoggable(Level.FINE)) { @@ -1376,8 +1382,12 @@ protected void processMaxActiveSwaps() { } try { swapOut(session); - } catch (Exception e) { + } catch (java.util.ConcurrentModificationException e1) { + // This is logged in writeSession() + } catch (IOException e) { // This is logged in writeSession() + } catch (Exception e) { + // This is logged in writeSession() } finally { session.unlockBackground(); } @@ -1398,8 +1408,8 @@ protected void processMaxIdleBackups() { if (!isStarted() || maxIdleBackup < 0) return; - final List sessions = findSessions(); - final long timeNow = System.currentTimeMillis(); + Session sessions[] = findSessions(); + long timeNow = System.currentTimeMillis(); // Back up all sessions idle longer than maxIdleBackup if (maxIdleBackup >= 0) { @@ -1417,8 +1427,12 @@ protected void processMaxIdleBackups() { } try { writeSession(session); - } catch (Exception e) { + } catch (java.util.ConcurrentModificationException e1) { + // This is logged in writeSession() + } catch (IOException e) { // This is logged in writeSession() + } catch (Exception e) { + // This is logged in writeSession() } finally { session.unlockBackground(); } @@ -1428,4 +1442,11 @@ protected void processMaxIdleBackups() { } } + + public String getMonitorAttributeValues() { + //FIXME if desired for monitoring 'file' + return ""; + } + + } diff --git a/appserver/web/web-core/src/main/java/org/apache/catalina/session/StandardManager.java b/appserver/web/web-core/src/main/java/org/apache/catalina/session/StandardManager.java index 9b338d4e002..ec4147ec289 100644 --- a/appserver/web/web-core/src/main/java/org/apache/catalina/session/StandardManager.java +++ b/appserver/web/web-core/src/main/java/org/apache/catalina/session/StandardManager.java @@ -669,23 +669,24 @@ public void writeSessions(OutputStream os, boolean doExpire) } // Write the number of active sessions, followed by the details - final StandardSession[] currentStandardSessions; + StandardSession[] currentStandardSessions = null; synchronized (sessions) { if (log.isLoggable(Level.FINE)) log.log(Level.FINE, "Unloading {0} sessions", sessions.size()); try { // START SJSAS 6375689 - for (final Session actSession : findSessions()) { + for (Session actSession : findSessions()) { StandardSession session = (StandardSession) actSession; session.passivate(); } // END SJSAS 6375689 - final List currentSessions = findSessions(); - int size = currentSessions.size(); + Session[] currentSessions = findSessions(); + int size = currentSessions.length; currentStandardSessions = new StandardSession[size]; oos.writeObject(size); for (int i = 0; i < size; i++) { - final StandardSession session = (StandardSession) currentSessions.get(i); + StandardSession session = + (StandardSession) currentSessions[i]; currentStandardSessions[i] = session; /* SJSAS 6375689 session.passivate(); @@ -881,19 +882,21 @@ public void stop(boolean isShutdown) throws LifecycleException { } // Expire all active sessions and notify their listeners - final List sessions = findSessions(); - for (final Session session : sessions) { - if (!session.isValid()) { - continue; - } - try { - session.expire(); - } catch (Throwable t) { - // Ignore - } finally { - // Measure against memory leaking if references to the session - // object are kept in a shared field somewhere - session.recycle(); + Session sessions[] = findSessions(); + if (sessions != null) { + for (Session session : sessions) { + if (!session.isValid()) { + continue; + } + try { + session.expire(); + } catch (Throwable t) { + // Ignore + } finally { + // Measure against memory leaking if references to the session + // object are kept in a shared field somewhere + session.recycle(); + } } } @@ -982,14 +985,16 @@ public void processExpires() { long timeNow = System.currentTimeMillis(); - final List sessions = findSessions(); - for (final Session session : sessions) { - final StandardSession sess = (StandardSession) session; - if (sess.lockBackground()) { - try { - sess.isValid(); - } finally { - sess.unlockBackground(); + Session[] sessions = findSessions(); + if (sessions != null) { + for (Session session : sessions) { + StandardSession sess = (StandardSession) session; + if (sess.lockBackground()) { + try { + sess.isValid(); + } finally { + sess.unlockBackground(); + } } } } diff --git a/appserver/web/web-core/src/main/java/org/apache/catalina/session/StandardSession.java b/appserver/web/web-core/src/main/java/org/apache/catalina/session/StandardSession.java index 2cef50b5f5d..0491da18ce9 100644 --- a/appserver/web/web-core/src/main/java/org/apache/catalina/session/StandardSession.java +++ b/appserver/web/web-core/src/main/java/org/apache/catalina/session/StandardSession.java @@ -791,7 +791,9 @@ public void addSessionListener(SessionListener listener) { */ @Override public void expire() { + expire(true); + } /** @@ -1690,7 +1692,7 @@ public void removeAttribute(String name, boolean notify, if (name == null) return; // Validate our current state - if (checkValid && !isValid()) + if (!isValid() && checkValid) throw new IllegalStateException ("removeAttribute: " + RESOURCE_BUNDLE.getString(LogFacade.SESSION_INVALIDATED_EXCEPTION)); diff --git a/appserver/web/web-glue/src/main/java/com/sun/enterprise/security/web/GlassFishSingleSignOn.java b/appserver/web/web-glue/src/main/java/com/sun/enterprise/security/web/GlassFishSingleSignOn.java index af0acfe67f4..746f7223169 100644 --- a/appserver/web/web-glue/src/main/java/com/sun/enterprise/security/web/GlassFishSingleSignOn.java +++ b/appserver/web/web-glue/src/main/java/com/sun/enterprise/security/web/GlassFishSingleSignOn.java @@ -37,25 +37,23 @@ * only if the new code is made subject to such option by the copyright * holder. */ -// Portions Copyright [2016-2020] [Payara Foundation and/or its affiliates] +// Portions Copyright [2016-2018] [Payara Foundation and/or its affiliates] package com.sun.enterprise.security.web; -import org.apache.catalina.HttpRequest; -import org.apache.catalina.LifecycleException; -import org.apache.catalina.Realm; -import org.apache.catalina.Request; -import org.apache.catalina.Response; -import org.apache.catalina.Session; -import org.apache.catalina.SessionEvent; +import org.apache.catalina.*; import org.apache.catalina.authenticator.Constants; import org.apache.catalina.authenticator.SingleSignOn; import org.apache.catalina.authenticator.SingleSignOnEntry; + import org.glassfish.web.LogFacade; +import javax.servlet.ServletException; import javax.servlet.http.Cookie; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import java.io.IOException; import java.util.ArrayList; +import java.util.Iterator; import java.util.concurrent.atomic.AtomicInteger; import java.util.logging.Level; import java.util.logging.Logger; @@ -110,12 +108,12 @@ public class GlassFishSingleSignOn extends SingleSignOn /** * Number of cache hits */ - private final AtomicInteger hitCount = new AtomicInteger(0); + private AtomicInteger hitCount = new AtomicInteger(0); /** * Number of cache misses */ - private final AtomicInteger missCount = new AtomicInteger(0); + private AtomicInteger missCount = new AtomicInteger(0); // ------------------------------------------------------------- Properties @@ -248,12 +246,18 @@ public void sessionEvent(SessionEvent event) { * * @param request The servlet request we are processing * @param response The servlet response we are creating + * @param context The valve context used to invoke the next valve in the current processing pipeline * - * @return the valve flag + * @exception IOException if an input/output error occurs + * @exception ServletException if a servlet error occurs + */ + /** + * IASRI 4665318 public void invoke(Request request, Response response, ValveContext context) throws IOException, + * ServletException { */ // START OF IASRI 4665318 @Override - public int invoke(final Request request, final Response response) { + public int invoke(Request request, Response response) throws IOException, ServletException { // END OF IASRI 4665318 // If this is not an HTTP request and response, just pass them on @@ -288,8 +292,7 @@ public int invoke(final Request request, final Response response) { if (logger.isLoggable(Level.FINE)) { logger.log(Level.FINE, LogFacade.CHECKING_SSO_COOKIE); } - - final Cookie[] cookies = hreq.getCookies(); + Cookie cookies[] = hreq.getCookies(); if (cookies == null) { return INVOKE_NEXT; } @@ -430,11 +433,14 @@ protected void deregister(String ssoId) { } // S1AS8 6155481 END // Look up and remove the corresponding SingleSignOnEntry - final SingleSignOnEntry sso = this.cache.remove(ssoId); - if (sso == null) { - return; + SingleSignOnEntry sso = null; + synchronized (cache) { + sso = (SingleSignOnEntry) cache.remove(ssoId); } + if (sso == null) + return; + // Expire any associated sessions sso.expireSessions(); @@ -458,24 +464,30 @@ private void processExpires() { long tooOld = System.currentTimeMillis() - ssoMaxInactive * 1000L; // S1AS8 6155481 START if (logger.isLoggable(Level.FINE)) { - logger.log(Level.FINE, LogFacade.SSO_EXPIRATION_STARTED, this.cache.size()); + logger.log(Level.FINE, LogFacade.SSO_EXPIRATION_STARTED, cache.size()); } // S1AS8 6155481 END - final ArrayList removals = new ArrayList<>(this.cache.size() / 2); + ArrayList removals = new ArrayList(cache.size() / 2); // build list of removal targets // Note that only those SSO entries which are NOT associated with - // any session are eligible for removal here. + // any session are elegible for removal here. // Currently no session association ever happens so this covers all // SSO entries. However, this should be addressed separately. try { - this.cache.forEach((ssoId, sso) -> { - if (sso.isEmpty() && sso.getLastAccessTime() < tooOld) { - removals.add(ssoId); + synchronized (cache) { + + Iterator it = cache.keySet().iterator(); + while (it.hasNext()) { + String key = it.next(); + SingleSignOnEntry sso = (SingleSignOnEntry) cache.get(key); + if (sso.isEmpty() && sso.getLastAccessTime() < tooOld) { + removals.add(key); + } } - }); + } int removalCount = removals.size(); // S1AS8 6155481 START @@ -483,13 +495,13 @@ private void processExpires() { logger.log(Level.FINE, LogFacade.SSO_CACHE_EXPIRE, removalCount); } // S1AS8 6155481 END - // deregister any eligible sso entries - for (final String removal : removals) { + // deregister any elegible sso entries + for (int i = 0; i < removalCount; i++) { // S1AS8 6155481 START if (logger.isLoggable(Level.FINE)) { - logger.log(Level.FINE, LogFacade.SSO_EXPRIRATION_REMOVING_ENTRY, removal); + logger.log(Level.FINE, LogFacade.SSO_EXPRIRATION_REMOVING_ENTRY, removals.get(i)); } - deregister(removal); + deregister(removals.get(i)); } // S1AS8 6155481 END } catch (Throwable e) { // don't let thread die @@ -596,7 +608,7 @@ protected void removeSession(String ssoId, Session session) { * @return Number of sessions participating in SSO */ public int getActiveSessionCount() { - return this.cache.size(); + return cache.size(); } /** diff --git a/appserver/web/web-ha/src/main/java/org/glassfish/web/ha/authenticator/HASingleSignOn.java b/appserver/web/web-ha/src/main/java/org/glassfish/web/ha/authenticator/HASingleSignOn.java index f8b036718d9..745333ca353 100644 --- a/appserver/web/web-ha/src/main/java/org/glassfish/web/ha/authenticator/HASingleSignOn.java +++ b/appserver/web/web-ha/src/main/java/org/glassfish/web/ha/authenticator/HASingleSignOn.java @@ -42,15 +42,17 @@ import com.sun.enterprise.container.common.spi.util.JavaEEIOUtils; import com.sun.enterprise.security.web.GlassFishSingleSignOn; + import org.apache.catalina.Session; import org.apache.catalina.authenticator.SingleSignOnEntry; import org.glassfish.ha.store.api.BackingStore; import org.glassfish.ha.store.api.BackingStoreException; import org.glassfish.web.ha.LogFacade; +import org.glassfish.web.ha.session.management.HAStoreBase; -import java.security.Principal; -import java.util.logging.Level; import java.util.logging.Logger; +import java.util.logging.Level; +import java.security.Principal; /** * @author Shing Wai Chan @@ -70,7 +72,7 @@ public HASingleSignOn(JavaEEIOUtils ioUtils, } @Override - protected void deregister(final String ssoId) { + protected void deregister(String ssoId) { //S1AS8 6155481 START if (logger.isLoggable(Level.FINE)) { @@ -78,16 +80,19 @@ protected void deregister(final String ssoId) { } //S1AS8 6155481 END // Look up and remove the corresponding SingleSignOnEntry - final SingleSignOnEntry sso = this.cache.remove(ssoId); - if (sso == null) { - return; + SingleSignOnEntry sso = null; + synchronized (cache) { + sso = cache.remove(ssoId); } + if (sso == null) + return; + // Expire any associated sessions sso.expireSessions(); try { - this.ssoEntryMetadataBackingStore.remove(ssoId); + ssoEntryMetadataBackingStore.remove(ssoId); } catch(BackingStoreException ex) { throw new IllegalStateException(ex); } @@ -105,23 +110,18 @@ protected void register(String ssoId, Principal principal, String authType, + "' with auth type '" + authType + "' and realmName '" + realmName + "'"); } - final HASingleSignOnEntry ssoEntry = new HASingleSignOnEntry( - ssoId, - principal, - authType, - username, - realmName, - // revisit maxIdleTime 1000000, version 0 - System.currentTimeMillis(), - 1000000, - 0, - ioUtils - ); - - this.cache.put(ssoId, ssoEntry); + HASingleSignOnEntry ssoEntry = null; + synchronized (cache) { + ssoEntry = new HASingleSignOnEntry(ssoId, principal, authType, + username, realmName, + // revisit maxIdleTime 1000000, version 0 + System.currentTimeMillis(), 1000000, 0, + ioUtils); + cache.put(ssoId, ssoEntry); + } try { - this.ssoEntryMetadataBackingStore.save(ssoId, ssoEntry.getMetadata(), true); + ssoEntryMetadataBackingStore.save(ssoId, ssoEntry.getMetadata(), true); } catch(BackingStoreException ex) { throw new IllegalStateException(ex); } @@ -151,21 +151,23 @@ public void associate(String ssoId, long ssoVersion, Session session) { } @Override - protected SingleSignOnEntry lookup(final String ssoId, final long ssoVersion) { + protected SingleSignOnEntry lookup(String ssoId, long ssoVersion) { SingleSignOnEntry ssoEntry = super.lookup(ssoId, ssoVersion); if (ssoEntry != null && ssoVersion > ssoEntry.getVersion()) { // clean the old cache - this.cache.remove(ssoId); + synchronized(cache) { + cache.remove(ssoId); + } ssoEntry = null; } - if (ssoEntry == null) { // load from ha store try { - final HASingleSignOnEntryMetadata mdata = this.ssoEntryMetadataBackingStore.load(ssoId, null); + HASingleSignOnEntryMetadata mdata = + ssoEntryMetadataBackingStore.load(ssoId, null); if (mdata != null) { ssoEntry = new HASingleSignOnEntry(getContainer(), mdata, ioUtils); - this.cache.put(ssoId, ssoEntry); + cache.put(ssoId, ssoEntry); } } catch(BackingStoreException ex) { throw new IllegalStateException(ex);