Fix DuplicateKeyException on concurrent JDBC session writes#3921
Open
fhanik wants to merge 3 commits into
Open
Fix DuplicateKeyException on concurrent JDBC session writes#3921fhanik wants to merge 3 commits into
fhanik wants to merge 3 commits into
Conversation
Two concurrent requests sharing a JSESSIONID can both load the session before either commits, both mark a new attribute as ADDED, and the second INSERT into SPRING_SESSION_ATTRIBUTES fails with DuplicateKeyException. Wire up a SessionRepositoryCustomizer per DB vendor so the attribute insert is idempotent: PostgreSqlJdbcIndexedSessionRepositoryCustomizer and MySqlJdbcIndexedSessionRepositoryCustomizer ship with Spring Session; add an HSQLDB counterpart using MERGE.
cf-foundation-community-automation
Bot
moved this to Inbox
in Foundational Infrastructure Working Group
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a production race condition in Spring Session JDBC where two concurrent requests sharing the same JSESSIONID can both attempt to INSERT the same (session_primary_id, attribute_name) into SPRING_SESSION_ATTRIBUTES, causing a DuplicateKeyException. The fix makes the attribute write idempotent by wiring a per-database SessionRepositoryCustomizer (PostgreSQL/MySQL provided by Spring Session; new HSQLDB customizer uses MERGE).
Changes:
- Add a
SessionRepositoryCustomizer<JdbcIndexedSessionRepository>bean to make session-attribute inserts vendor-specific UPSERTs. - Introduce an HSQLDB-specific
JdbcIndexedSessionRepositorycustomizer that usesMERGEfor idempotent attribute writes. - Add regression tests that deterministically reproduce the “double-INSERT” race and assert last-write-wins behavior across DBs.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
uaa/src/test/java/org/cloudfoundry/identity/uaa/db/JdbcSessionConcurrentWriteMockMvcTest.java |
Integration-style regression test using the real UAA context with DB-backed sessions enabled. |
server/src/test/java/org/cloudfoundry/identity/uaa/web/beans/JdbcSessionConcurrentWriteTest.java |
Repository-level regression test that applies the same per-platform customizer logic as production. |
server/src/main/java/org/cloudfoundry/identity/uaa/web/beans/UaaJdbcSessionConfig.java |
Wires a per-DB SessionRepositoryCustomizer bean to make attribute inserts idempotent. |
server/src/main/java/org/cloudfoundry/identity/uaa/web/beans/HsqldbJdbcIndexedSessionRepositoryCustomizer.java |
Adds HSQLDB UPSERT behavior for SPRING_SESSION_ATTRIBUTES via MERGE. |
Comment on lines
+3
to
+16
| import java.util.Collections; | ||
|
|
||
| import org.cloudfoundry.identity.uaa.DefaultTestContext; | ||
| import org.cloudfoundry.identity.uaa.web.beans.UaaJdbcSessionConfig; | ||
| import org.junit.jupiter.api.AfterEach; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.springframework.beans.factory.annotation.Autowired; | ||
| import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; | ||
| import org.springframework.security.core.context.SecurityContext; | ||
| import org.springframework.security.core.context.SecurityContextImpl; | ||
| import org.springframework.session.Session; | ||
| import org.springframework.session.SessionRepository; | ||
| import org.springframework.session.jdbc.JdbcIndexedSessionRepository; | ||
| import org.springframework.test.context.TestPropertySource; |
| */ | ||
| @DefaultTestContext | ||
| @TestPropertySource(properties = "servlet.session-store=database") | ||
| class JdbcSessionConcurrentWriteMockMvcTest { |
Comment on lines
+3
to
+22
| import java.util.Collections; | ||
| import javax.sql.DataSource; | ||
|
|
||
| import org.cloudfoundry.identity.uaa.annotations.WithDatabaseContext; | ||
| import org.cloudfoundry.identity.uaa.db.beans.DatabaseProperties; | ||
| import org.junit.jupiter.api.BeforeEach; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.springframework.beans.factory.annotation.Autowired; | ||
| import org.springframework.jdbc.core.JdbcTemplate; | ||
| import org.springframework.jdbc.datasource.DataSourceTransactionManager; | ||
| import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; | ||
| import org.springframework.security.core.context.SecurityContext; | ||
| import org.springframework.security.core.context.SecurityContextImpl; | ||
| import org.springframework.session.Session; | ||
| import org.springframework.session.SessionRepository; | ||
| import org.springframework.session.config.SessionRepositoryCustomizer; | ||
| import org.springframework.session.jdbc.JdbcIndexedSessionRepository; | ||
| import org.springframework.session.jdbc.MySqlJdbcIndexedSessionRepositoryCustomizer; | ||
| import org.springframework.session.jdbc.PostgreSqlJdbcIndexedSessionRepositoryCustomizer; | ||
| import org.springframework.transaction.support.TransactionTemplate; |
Comment on lines
+90
to
+92
| * <li>Request A's {@code save()} inserts the attribute row — succeeds.</li> | ||
| * <li>Request B's {@code save()} insert the same attribute row — currently | ||
| * throws {@code DuplicateKeyException}.</li> |
Comment on lines
+54
to
+59
| @Autowired | ||
| private DatabaseProperties databaseProperties; | ||
|
|
||
| @SuppressWarnings("rawtypes") | ||
| private SessionRepository repository; | ||
|
|
Comment on lines
+98
to
+101
| Session created = repository.createSession(); | ||
| repository.save(created); | ||
| String sessionId = created.getId(); | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two concurrent requests sharing a JSESSIONID can both load the session before either commits, both mark a new attribute as ADDED, and the second INSERT into SPRING_SESSION_ATTRIBUTES fails with DuplicateKeyException. Wire up a SessionRepositoryCustomizer per DB vendor so the attribute insert is idempotent: PostgreSqlJdbcIndexedSessionRepositoryCustomizer and MySqlJdbcIndexedSessionRepositoryCustomizer ship with Spring Session; add an HSQLDB counterpart using MERGE.