Skip to content

Commit 19bcf52

Browse files
jamesfredleyjamesfredley
authored
Merge branch '8.0.x' into fix/15374-codec-metaclass-registration-overhead
2 parents b9f5c1f + e929338 commit 19bcf52

7 files changed

Lines changed: 236 additions & 46 deletions

File tree

‎grails-controllers/src/main/groovy/grails/artefact/Controller.groovy‎

Lines changed: 6 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -286,8 +286,7 @@ trait Controller implements ResponseRenderer, ResponseRedirector, RequestForward
286286
@Generated
287287
TokenResponseHandler withForm(GrailsWebRequest webRequest, Closure callable) {
288288
TokenResponseHandler handler
289-
if (isTokenValid(webRequest)) {
290-
resetToken(webRequest)
289+
if (consumeToken(webRequest)) {
291290
handler = new ValidResponseHandler(callable?.call())
292291
}
293292
else {
@@ -303,7 +302,7 @@ trait Controller implements ResponseRenderer, ResponseRedirector, RequestForward
303302
*
304303
* @param request The servlet request
305304
*/
306-
private synchronized boolean isTokenValid(GrailsWebRequest webRequest) {
305+
private boolean consumeToken(GrailsWebRequest webRequest) {
307306
final request = webRequest.getCurrentRequest()
308307
SynchronizerTokensHolder tokensHolderInSession = (SynchronizerTokensHolder) request.getSession(false)?.getAttribute(SynchronizerTokensHolder.HOLDER)
309308
if (!tokensHolderInSession) return false
@@ -314,27 +313,11 @@ trait Controller implements ResponseRenderer, ResponseRedirector, RequestForward
314313
String urlInRequest = webRequest.params[SynchronizerTokensHolder.TOKEN_URI]
315314
if (!urlInRequest) return false
316315

317-
try {
318-
return tokensHolderInSession.isValid(urlInRequest, tokenInRequest)
319-
}
320-
catch (IllegalArgumentException) {
321-
return false
322-
}
323-
}
324-
325-
/**
326-
* Resets the token in the request
327-
*/
328-
private synchronized resetToken(GrailsWebRequest webRequest) {
329-
final request = webRequest.getCurrentRequest()
330-
SynchronizerTokensHolder tokensHolderInSession = (SynchronizerTokensHolder) request.getSession(false)?.getAttribute(SynchronizerTokensHolder.HOLDER)
331-
String urlInRequest = webRequest.params[SynchronizerTokensHolder.TOKEN_URI]
332-
String tokenInRequest = webRequest.params[SynchronizerTokensHolder.TOKEN_KEY]
333-
334-
if (urlInRequest && tokenInRequest) {
335-
tokensHolderInSession.resetToken(urlInRequest, tokenInRequest)
316+
boolean valid = tokensHolderInSession.isValidAndResetToken(urlInRequest, tokenInRequest)
317+
if (tokensHolderInSession.isEmpty()) {
318+
request.getSession(false)?.removeAttribute(SynchronizerTokensHolder.HOLDER)
336319
}
337-
if (tokensHolderInSession.isEmpty()) request.getSession(false)?.removeAttribute(SynchronizerTokensHolder.HOLDER)
320+
return valid
338321
}
339322

340323
@Generated

‎grails-doc/src/en/guide/theWebLayer/controllers/formtokens.adoc‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,6 @@ If you only provide the link:{controllersRef}withForm.html[withForm] method and
4444
</g:if>
4545
----
4646

47+
Tokens are single-use. When `withForm` validates a token, Grails consumes that token atomically, so duplicate concurrent submissions for the same token allow exactly one request to run the valid block and route the rest to `invalidToken`. Token state is stored in the session and bounded to 100 form URL buckets, with at most 100 tokens per URL. If either limit is exceeded, Grails drops the oldest URL bucket or oldest token for that URL. Applications that keep many unsubmitted forms open in one session can therefore expire the oldest tokens and receive `invalidToken` when those forms are submitted later.
48+
4749
WARNING: The link:{controllersRef}withForm.html[withForm] tag makes use of the link:{controllersRef}session.html[session] and hence requires session affinity or clustered sessions if used in a cluster.

‎grails-doc/src/en/ref/Controllers/withForm.adoc‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,4 +62,6 @@ withForm {
6262
}
6363
----
6464

65+
Tokens are single-use. A valid token is consumed atomically when `withForm` validates it, so duplicate concurrent submissions for the same token allow exactly one request to run the valid block. Grails stores token state in the session with a limit of 100 form URL buckets and 100 tokens per URL; after those limits, the oldest bucket or oldest token is discarded and later submission of that form is handled as `invalidToken`.
66+
6567
See link:{guidePath}theWebLayer.html#formtokens[Handling Duplicate Form Submissions] for more information.

‎grails-test-examples/demo33/src/test/groovy/demo/TestControllerSpec.groovy‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,13 @@ class TestControllerSpec extends Specification implements ControllerUnitTest<Tes
157157

158158
then:
159159
"Good" == response.text
160+
161+
when:
162+
response.reset()
163+
controller.renderWithForm()
164+
165+
then:
166+
"Bad" == response.text
160167
}
161168

162169
void 'test file upload'() {

‎grails-test-examples/hibernate7/demo33/src/test/groovy/demo/TestControllerSpec.groovy‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,13 @@ class TestControllerSpec extends Specification implements ControllerUnitTest<Tes
157157

158158
then:
159159
"Good" == response.text
160+
161+
when:
162+
response.reset()
163+
controller.renderWithForm()
164+
165+
then:
166+
"Bad" == response.text
160167
}
161168

162169
void 'test file upload'() {

‎grails-test-suite-web/src/test/groovy/org/grails/web/servlet/mvc/SynchronizerTokensHolderTests.groovy‎

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@
1818
*/
1919
package org.grails.web.servlet.mvc
2020

21+
import java.util.concurrent.Callable
22+
import java.util.concurrent.CountDownLatch
23+
import java.util.concurrent.Executors
24+
import java.util.concurrent.Future
25+
2126
import org.junit.jupiter.api.Test
2227

2328
import static org.junit.jupiter.api.Assertions.*
@@ -73,6 +78,85 @@ class SynchronizerTokensHolderTests {
7378
assertEquals 1, holder.currentTokens[url2].size()
7479
}
7580

81+
@Test
82+
void testGeneratedTokensAreBoundedByUrlCount() {
83+
SynchronizerTokensHolder holder = new SynchronizerTokensHolder()
84+
85+
String firstUrl = 'url0'
86+
String firstToken = holder.generateToken(firstUrl)
87+
(1..SynchronizerTokensHolder.DEFAULT_MAX_TOKEN_URLS).each { Integer index ->
88+
holder.generateToken("url${index}".toString())
89+
}
90+
91+
assertEquals SynchronizerTokensHolder.DEFAULT_MAX_TOKEN_URLS, holder.currentTokens.size()
92+
assertFalse holder.isValid(firstUrl, firstToken)
93+
}
94+
95+
@Test
96+
void testExistingOversizedUrlMapIsTrimmedWhenGeneratingToken() {
97+
SynchronizerTokensHolder holder = new SynchronizerTokensHolder()
98+
String firstUrl = 'url0'
99+
holder.currentTokens[firstUrl] = [UUID.randomUUID()] as LinkedHashSet<UUID>
100+
(1..(SynchronizerTokensHolder.DEFAULT_MAX_TOKEN_URLS + 5)).each { Integer index ->
101+
holder.currentTokens["url${index}".toString()] = [UUID.randomUUID()] as LinkedHashSet<UUID>
102+
}
103+
104+
String newToken = holder.generateToken('new-url')
105+
106+
assertEquals SynchronizerTokensHolder.DEFAULT_MAX_TOKEN_URLS, holder.currentTokens.size()
107+
assertFalse holder.currentTokens.containsKey(firstUrl)
108+
assertTrue holder.isValid('new-url', newToken)
109+
}
110+
111+
@Test
112+
void testExistingOversizedUrlMapKeepsUrlReceivingNewToken() {
113+
SynchronizerTokensHolder holder = new SynchronizerTokensHolder()
114+
String refreshedUrl = 'url1'
115+
holder.currentTokens['url0'] = [UUID.randomUUID()] as LinkedHashSet<UUID>
116+
holder.currentTokens[refreshedUrl] = [UUID.randomUUID()] as LinkedHashSet<UUID>
117+
(2..(SynchronizerTokensHolder.DEFAULT_MAX_TOKEN_URLS + 5)).each { Integer index ->
118+
holder.currentTokens["url${index}".toString()] = [UUID.randomUUID()] as LinkedHashSet<UUID>
119+
}
120+
121+
String newToken = holder.generateToken(refreshedUrl)
122+
123+
assertEquals SynchronizerTokensHolder.DEFAULT_MAX_TOKEN_URLS, holder.currentTokens.size()
124+
assertTrue holder.currentTokens.containsKey(refreshedUrl)
125+
assertTrue holder.isValid(refreshedUrl, newToken)
126+
}
127+
128+
@Test
129+
void testGeneratedTokensAreBoundedPerUrl() {
130+
SynchronizerTokensHolder holder = new SynchronizerTokensHolder()
131+
String url = 'url1'
132+
String firstToken = holder.generateToken(url)
133+
String lastToken = null
134+
(1..SynchronizerTokensHolder.DEFAULT_MAX_TOKENS_PER_URL).each {
135+
lastToken = holder.generateToken(url)
136+
}
137+
138+
assertEquals SynchronizerTokensHolder.DEFAULT_MAX_TOKENS_PER_URL, holder.currentTokens[url].size()
139+
assertFalse holder.isValid(url, firstToken)
140+
assertTrue holder.isValid(url, lastToken)
141+
}
142+
143+
@Test
144+
void testExistingOversizedTokenSetIsTrimmedWhenGeneratingToken() {
145+
SynchronizerTokensHolder holder = new SynchronizerTokensHolder()
146+
String url = 'url1'
147+
UUID firstToken = UUID.randomUUID()
148+
holder.currentTokens[url] = [firstToken] as LinkedHashSet<UUID>
149+
(1..(SynchronizerTokensHolder.DEFAULT_MAX_TOKENS_PER_URL + 5)).each {
150+
holder.currentTokens[url].add(UUID.randomUUID())
151+
}
152+
153+
String newToken = holder.generateToken(url)
154+
155+
assertEquals SynchronizerTokensHolder.DEFAULT_MAX_TOKENS_PER_URL, holder.currentTokens[url].size()
156+
assertFalse holder.currentTokens[url].contains(firstToken)
157+
assertTrue holder.isValid(url, newToken)
158+
}
159+
76160
@Test
77161
void testIsValid() {
78162
SynchronizerTokensHolder holder = new SynchronizerTokensHolder()
@@ -85,6 +169,67 @@ class SynchronizerTokensHolderTests {
85169
assertFalse holder.isValid(url, token + '!')
86170
}
87171

172+
@Test
173+
void testValidTokenCanOnlyBeConsumedOnce() {
174+
SynchronizerTokensHolder holder = new SynchronizerTokensHolder()
175+
String url = 'url1'
176+
String token = holder.generateToken(url)
177+
178+
assertTrue holder.isValidAndResetToken(url, token)
179+
assertFalse holder.isValidAndResetToken(url, token)
180+
assertTrue holder.empty
181+
}
182+
183+
@Test
184+
void testInvalidTokensAreNotConsumed() {
185+
SynchronizerTokensHolder holder = new SynchronizerTokensHolder()
186+
String url = 'url1'
187+
String token = holder.generateToken(url)
188+
189+
assertFalse holder.isValidAndResetToken(url, null)
190+
assertFalse holder.isValidAndResetToken(url, '')
191+
assertFalse holder.isValidAndResetToken(url, token + '!')
192+
assertTrue holder.isValidAndResetToken(url, token)
193+
}
194+
195+
@Test
196+
void testConsumingOneTokenKeepsSiblingTokensForSameUrl() {
197+
SynchronizerTokensHolder holder = new SynchronizerTokensHolder()
198+
String url = 'url1'
199+
String token1 = holder.generateToken(url)
200+
String token2 = holder.generateToken(url)
201+
202+
assertTrue holder.isValidAndResetToken(url, token1)
203+
assertFalse holder.isValidAndResetToken(url, token1)
204+
assertTrue holder.isValidAndResetToken(url, token2)
205+
assertTrue holder.empty
206+
}
207+
208+
@Test
209+
void testConcurrentTokenConsumeAllowsOneSuccess() {
210+
SynchronizerTokensHolder holder = new SynchronizerTokensHolder()
211+
String url = 'url1'
212+
String token = holder.generateToken(url)
213+
CountDownLatch start = new CountDownLatch(1)
214+
def executor = Executors.newFixedThreadPool(2)
215+
216+
try {
217+
List<Future<Boolean>> results = (1..2).collect {
218+
executor.submit({ ->
219+
start.await()
220+
holder.isValidAndResetToken(url, token)
221+
} as Callable<Boolean>)
222+
}
223+
start.countDown()
224+
225+
assertEquals 1, results.count { Future<Boolean> result -> result.get() }
226+
assertTrue holder.empty
227+
}
228+
finally {
229+
executor.shutdownNow()
230+
}
231+
}
232+
88233
@Test
89234
void testResetTokens() {
90235
SynchronizerTokensHolder holder = new SynchronizerTokensHolder()

‎grails-web-mvc/src/main/groovy/org/grails/web/servlet/mvc/SynchronizerTokensHolder.groovy‎

Lines changed: 67 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@
1818
*/
1919
package org.grails.web.servlet.mvc
2020

21-
import java.util.concurrent.CopyOnWriteArraySet
22-
2321
import jakarta.servlet.http.HttpSession
2422

2523
/**
@@ -35,51 +33,97 @@ class SynchronizerTokensHolder implements Serializable {
3533
public static final String HOLDER = 'SYNCHRONIZER_TOKENS_HOLDER'
3634
public static final String TOKEN_KEY = 'SYNCHRONIZER_TOKEN'
3735
public static final String TOKEN_URI = 'SYNCHRONIZER_URI'
36+
public static final int DEFAULT_MAX_TOKEN_URLS = 100
37+
public static final int DEFAULT_MAX_TOKENS_PER_URL = 100
3838

39-
Map<String, Set<UUID>> currentTokens = [:]
39+
Map<String, Set<UUID>> currentTokens = new LinkedHashMap<String, Set<UUID>>()
4040

41-
boolean isValid(String url, String token) {
42-
try {
43-
getTokens(url).contains(UUID.fromString(token))
44-
}
45-
catch (IllegalArgumentException e) {
46-
false
47-
}
41+
synchronized boolean isValid(String url, String token) {
42+
UUID uuid = parseToken(token)
43+
uuid != null && getExistingTokens(url)?.contains(uuid)
4844
}
4945

50-
String generateToken(String url) {
46+
synchronized String generateToken(String url) {
5147
final UUID uuid = UUID.randomUUID()
52-
getTokens(url).add(uuid)
48+
Set<UUID> tokens = getTokens(url)
49+
tokens.add(uuid)
50+
removeEldestTokenIfNecessary(tokens)
51+
removeEldestTokenUrlIfNecessary()
5352
return uuid
5453
}
5554

56-
void resetToken(String url) {
55+
synchronized void resetToken(String url) {
5756
currentTokens.remove(url)
5857
}
5958

60-
void resetToken(String url, String token) {
59+
synchronized void resetToken(String url, String token) {
6160
if (url && token) {
62-
final Set set = getTokens(url)
63-
try {
64-
set.remove(UUID.fromString(token))
61+
final Set<UUID> set = getExistingTokens(url)
62+
UUID uuid = parseToken(token)
63+
if (uuid != null) {
64+
set?.remove(uuid)
6565
}
66-
catch (IllegalArgumentException ignored) {}
67-
if (set.isEmpty()) {
66+
if (set?.isEmpty()) {
6867
currentTokens.remove(url)
6968
}
7069
}
7170
}
7271

73-
boolean isEmpty() {
72+
synchronized boolean isValidAndResetToken(String url, String token) {
73+
UUID uuid = parseToken(token)
74+
if (uuid == null) {
75+
return false
76+
}
77+
78+
final Set<UUID> set = getExistingTokens(url)
79+
boolean valid = set != null && set.remove(uuid)
80+
if (set?.isEmpty()) {
81+
currentTokens.remove(url)
82+
}
83+
valid
84+
}
85+
86+
synchronized boolean isEmpty() {
7487
return currentTokens.isEmpty() || currentTokens.every { String url, Set<UUID> uuids -> uuids.isEmpty() }
7588
}
7689

7790
protected Set<UUID> getTokens(String url) {
78-
if (!currentTokens.containsKey(url)) {
79-
currentTokens[url] = new CopyOnWriteArraySet<UUID>()
91+
Set<UUID> tokens = currentTokens.remove(url)
92+
if (tokens == null) {
93+
tokens = new LinkedHashSet<UUID>()
94+
}
95+
96+
currentTokens[url] = tokens
97+
tokens
98+
}
99+
100+
private Set<UUID> getExistingTokens(String url) {
101+
url ? currentTokens[url] : null
102+
}
103+
104+
private UUID parseToken(String token) {
105+
if (!token) {
106+
return null
107+
}
108+
109+
try {
110+
UUID.fromString(token)
80111
}
112+
catch (IllegalArgumentException ignored) {
113+
null
114+
}
115+
}
116+
117+
private void removeEldestTokenUrlIfNecessary() {
118+
while (currentTokens.size() > DEFAULT_MAX_TOKEN_URLS) {
119+
currentTokens.remove(currentTokens.keySet().iterator().next())
120+
}
121+
}
81122

82-
currentTokens[url]
123+
private void removeEldestTokenIfNecessary(Set<UUID> tokens) {
124+
while (tokens.size() > DEFAULT_MAX_TOKENS_PER_URL) {
125+
tokens.remove(tokens.iterator().next())
126+
}
83127
}
84128

85129
static SynchronizerTokensHolder store(HttpSession session) {

0 commit comments

Comments
 (0)