Resolve URL mapping names from the match rather than the request

grails-doc/src/en/guide/theWebLayer/urlmappings/embeddedVariables.adoc

@@ -72,6 +72,8 @@ static mappings = {
 
 Here the name of the controller, action and id are implicitly obtained from the variables `controller`, `action` and `id` embedded within the URL.
 
+These names come from the URL and from nowhere else. A request parameter named `controller`, `action` or `namespace` does not stand in for a token the URL leaves out: given the mapping above, `/product?action=delete` routes to `ProductController`'s default action, not to `delete`. The parameter is still bound, and is still readable as `params.action`; it just has no say in which action runs. To route on a request parameter, ask for it explicitly with a closure, as shown at the end of this section.
+
 ===== Wildcard Artefact Validation
 
 By default, when URL mappings use wildcard variables like `$controller`, `$action`, or `$namespace`, Grails validates the captured values against registered controller artefacts. If a captured value does not correspond to a real controller, action, or namespace, the mapping is skipped and the next mapping is tried.

grails-doc/src/en/guide/upgrading/upgrading80x.adoc

@@ -2917,3 +2917,65 @@ job is disabled with `jobEnabled = false`, or the class is not a job artefact of
 
 An application that caught `NullPointerException` around these calls has to catch those instead. See
 <<quartzDynamicScheduling,Dynamic Job Scheduling>>.
+
+==== 52. URL Mapping Names Come From the URI, Not From Request Parameters
+
+A mapping that captures its controller, action or namespace from the URL — `"/$controller/$action?/$id?"`,
+the default mapping every application starts with — used to resolve those names by reading the request's
+parameters during dispatch. The values were normally the ones the URI supplied, because the match wrote its
+captures over the request parameters first. But a token the URI did *not* supply left whatever the request
+carried under that name in place, and the request's own value decided the route.
+
+A request for `/article?action=gallery` therefore reached `ArticleController.gallery()` instead of the
+controller's default action: the optional `$action` token captured nothing, and the query string filled the
+gap. The same held for `controller` and `namespace`, and for a form body as well as a query string.
+
+Captured names now come from the match itself. A token the URI does not supply is simply absent, and a
+request parameter of the same name has no say in routing:
+
+[source,groovy]
+.grails-app/controllers/UrlMappings.groovy
+----
+static mappings = {
+    "/$controller/$action?/$id?"()
+}
+----
+
+[cols="1,1,1", options="header"]
+|===
+| Request
+| Grails 7
+| Grails 8
+
+| `/article/gallery`
+| `ArticleController.gallery()`
+| `ArticleController.gallery()`
+
+| `/article?action=gallery`
+| `ArticleController.gallery()`
+| the default action of `ArticleController`
+|===
+
+`params.action` still holds `gallery` in both versions — only the routing decision changed.
+
+Choosing an action from a request parameter is still supported, by asking for it explicitly with a closure:
+
+[source,groovy]
+.grails-app/controllers/UrlMappings.groovy
+----
+static mappings = {
+    "/$controller" {
+        action = { params.goHere }
+    }
+}
+----
+
+A closure supplied by the mapping keeps reading request state, exactly as before, and mappings that use one
+are unaffected by this change.
+
+Two consequences beyond the routing fix. Request parameters are now built once per request rather than once
+per candidate mapping, since candidates no longer have to be configured onto the request to be identified.
+And `grails.web.mapping.UrlMappingInfo` gains a
+`isNameResolutionRequestDependent()` method for callers that resolve names themselves; it is a default
+method that reports `true`, so an implementation outside the framework keeps its current behaviour without
+changes.

grails-test-suite-web/src/test/groovy/org/grails/web/mapping/UrlMappingParameterTests.groovy

@@ -45,12 +45,12 @@ class UrlMappingParameterTests extends Specification implements UrlMappingsUnitT
     }
 
     void testNotEqual() {
-        when:
+        when: 'a request parameter shadows the name the URI captures'
         webRequest.params.controller = 'foo'
         def info = urlMappingsHolder.match('/showSomething/bad')
 
-        then:'url should not have matched'
-        info.controllerName == 'foo'
+        then: 'the notEqual constraint rejects the blog mapping and the URI falls through to the default mapping, whose controller is the one the URI captured rather than the request parameter'
+        info.controllerName == 'showSomething'
 
         when:
         info = urlMappingsHolder.match('/showSomething/good')

grails-web-url-mappings/src/main/groovy/grails/web/mapping/UrlMappingInfo.java

@@ -147,4 +147,25 @@ public interface UrlMappingInfo {
     default boolean hasWildcardCaptures() {
         return false;
     }
+
+    /**
+     * Whether resolving {@link #getControllerName()}, {@link #getActionName()},
+     * {@link #getNamespace()} or {@link #getViewName()} depends on the state of the current request,
+     * and so requires {@link #configure(GrailsWebRequest)} to have run first.
+     *
+     * <p>A mapping such as <code>"/$controller/$action?"</code> captures its names from the URI and can
+     * answer them from the match alone. A mapping that computes a name with a closure of its own - for
+     * example <code>"/$controller" { action = { params.goHere } }</code> - reads whatever that closure
+     * reaches for, typically the parameters of the current request, and a caller that wants the name
+     * has to configure the request before asking for it.</p>
+     *
+     * <p>Implementations that cannot tell should leave this as the default, which asks callers to
+     * configure the request and preserves the behaviour of every release before this method existed.</p>
+     *
+     * @return true if the names can only be resolved once the request has been configured
+     * @since 8.0
+     */
+    default boolean isNameResolutionRequestDependent() {
+        return true;
+    }
 }

grails-web-url-mappings/src/main/groovy/org/grails/web/mapping/AbstractUrlMappingInfo.java

@@ -120,6 +120,9 @@ protected String evaluateNameForValue(Object value) {
         if (value instanceof CharSequence) {
             return value.toString().trim();
         }
+        else if (value instanceof RuntimeConstraintEvaluator) {
+            return evaluateCapturedName((RuntimeConstraintEvaluator) value);
+        }
         else {
             GrailsWebRequest webRequest = (GrailsWebRequest) RequestContextHolder.getRequestAttributes();
             return evaluateNameForValue(value, webRequest);
@@ -131,6 +134,10 @@ protected String evaluateNameForValue(Object value, GrailsWebRequest webRequest)
             return null;
         }
 
+        if (value instanceof RuntimeConstraintEvaluator) {
+            return evaluateCapturedName((RuntimeConstraintEvaluator) value);
+        }
+
         String name;
         if (value instanceof Closure) {
             Closure callable = (Closure) value;
@@ -150,6 +157,20 @@ else if (value instanceof Map) {
         return name != null ? name.trim() : null;
     }
 
+    /**
+     * Resolves a name the mapping captured from the URI - the {@code $controller} token of
+     * {@code "/$controller/$action?"}, for example - from this instance's own parameters, so that the
+     * answer depends only on the mapping and the URI that matched it and not on what the current
+     * request happens to carry.
+     *
+     * @param evaluator The evaluator held by the mapping for the token
+     * @return The captured value, or null if the URI did not supply one
+     */
+    private String evaluateCapturedName(RuntimeConstraintEvaluator evaluator) {
+        Object value = params.get(evaluator.getConstraintName());
+        return value != null ? value.toString().trim() : null;
+    }
+
     /**
      * The redirect information should be a String or a Map.  If it
      * is a String that string is the URI to redirect to.  If it is

grails-web-url-mappings/src/main/groovy/org/grails/web/mapping/DefaultUrlMappingInfo.java

@@ -212,6 +212,25 @@ public boolean hasWildcardCaptures() {
                 namespace instanceof Closure;
     }
 
+    @Override
+    public boolean isNameResolutionRequestDependent() {
+        return isRequestDependent(controllerName) || isRequestDependent(actionName) ||
+                isRequestDependent(namespace) || isRequestDependent(viewName);
+    }
+
+    /**
+     * A name captured from the URI is held by this instance, so resolving it needs nothing from the
+     * request. A name computed by a closure the mapping supplied reads whatever that closure reaches
+     * for, which is typically the parameters of the current request. A name selected by HTTP method
+     * reads the method from the request directly, which configuring it does not affect.
+     *
+     * @param name The controller, action, namespace or view name held by this instance
+     * @return true if resolving the name needs the request to have been configured
+     */
+    private static boolean isRequestDependent(Object name) {
+        return name instanceof Closure && !(name instanceof RuntimeConstraintEvaluator);
+    }
+
     public String getViewName() {
         return evaluateNameForValue(viewName);
     }

grails-web-url-mappings/src/main/groovy/org/grails/web/mapping/RegexUrlMapping.java

@@ -34,8 +34,6 @@
 import java.util.regex.Pattern;
 import java.util.regex.PatternSyntaxException;
 
-import groovy.lang.Closure;
-
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -732,7 +730,11 @@ else if (viewName != null && controllerName == null) {
 
     /**
      * This method will look for a constraint for the given name and return a closure that when executed will
-     * attempt to evaluate its value from the bound request parameters at runtime.
+     * attempt to evaluate its value at runtime.
+     *
+     * <p>The mapping is shared by every request it matches, so the returned evaluator holds only the name
+     * of the token to resolve. The {@link UrlMappingInfo} produced by a match resolves it against the values
+     * that match captured.</p>
      *
      * @param name        The name of the constrained property
      * @param constraints The array of current ConstrainedProperty instances
@@ -743,15 +745,7 @@ private Object createRuntimeConstraintEvaluator(final String name, ConstrainedPr
 
         for (ConstrainedProperty constraint : constraints) {
             if (constraint.getPropertyName().equals(name)) {
-                return new Closure(this) {
-                    private static final long serialVersionUID = -2404119898659287216L;
-
-                    @Override
-                    public Object call(Object... objects) {
-                        GrailsWebRequest webRequest = (GrailsWebRequest) RequestContextHolder.currentRequestAttributes();
-                        return webRequest.getParams().get(name);
-                    }
-                };
+                return new RuntimeConstraintEvaluator(this, name);
             }
         }
         return null;

grails-web-url-mappings/src/main/groovy/org/grails/web/mapping/RuntimeConstraintEvaluator.java

@@ -0,0 +1,67 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    https://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package org.grails.web.mapping;
+
+import groovy.lang.Closure;
+
+import org.springframework.web.context.request.RequestContextHolder;
+
+import org.grails.web.servlet.mvc.GrailsWebRequest;
+
+/**
+ * Resolves the value a URL mapping captured for one of its named tokens - the {@code $controller},
+ * {@code $action} or {@code $namespace} segments of a mapping such as
+ * {@code "/$controller/$action?/$id?"}.
+ *
+ * <p>A {@link RegexUrlMapping} is built once and shared by every request it matches, so an evaluator
+ * held by the mapping cannot hold the captured value itself. {@link AbstractUrlMappingInfo}, which is
+ * created per match and does hold it, recognises this type and resolves the name straight from its own
+ * parameters. That keeps {@link grails.web.mapping.UrlMappingInfo#getControllerName()} and its siblings
+ * a function of the mapping and the matched URI, rather than of whatever happens to be bound to the
+ * current thread.</p>
+ *
+ * <p>Calling the closure directly falls back to the parameters of the current request, which is all a
+ * shared instance can do on its own.</p>
+ *
+ * @since 8.0
+ */
+class RuntimeConstraintEvaluator extends Closure<Object> {
+
+    private static final long serialVersionUID = -2404119898659287216L;
+
+    private final String constraintName;
+
+    RuntimeConstraintEvaluator(Object owner, String constraintName) {
+        super(owner);
+        this.constraintName = constraintName;
+    }
+
+    /**
+     * @return the name of the constrained property this evaluator resolves
+     */
+    String getConstraintName() {
+        return constraintName;
+    }
+
+    @Override
+    public Object call(Object... args) {
+        GrailsWebRequest webRequest = (GrailsWebRequest) RequestContextHolder.currentRequestAttributes();
+        return webRequest.getParams().get(constraintName);
+    }
+}

grails-web-url-mappings/src/main/groovy/org/grails/web/mapping/mvc/AbstractGrailsControllerUrlMappings.groovy

@@ -216,7 +216,11 @@ abstract class AbstractGrailsControllerUrlMappings implements UrlMappings {
                 matches.add(info)
                 continue
             }
-            if (webRequest != null) {
+            // Names captured from the URI resolve from the match itself, so the candidate can be
+            // identified without touching the request. Only a mapping that computes a name from
+            // request state needs its parameters built first, and only that mapping pays for it -
+            // the winner's parameters are built once, by UrlMappingsHandlerMapping.
+            if (webRequest != null && info.isNameResolutionRequestDependent()) {
                 webRequest.resetParams()
                 info.configure(webRequest)
             }