jamesfredley opened a new pull request, #15398:
URL: https://github.com/apache/grails-core/pull/15398

   ## Summary
   
   GSP declaration blocks (`<%! ... %>` and `!{ ... }!`) have never worked 
correctly in any Grails version. The `declare()` method in 
`GroovyPageParser.java` generates broken Groovy code that causes a 500 Internal 
Server Error at runtime with no useful diagnostic. This fix replaces the broken 
implementation with a clear compile-time `GrailsTagException` that tells the 
developer exactly what syntax is unsupported and suggests alternatives.
   
   ## Bug Description
   
   When a GSP contains a JSP-style declaration block:
   
   ```gsp
   <%! int counter = 0; %>
   ```
   
   The application compiles and starts normally, but rendering the page at 
runtime produces a 500 Internal Server Error. The error message gives no 
indication that the declaration block syntax is the cause.
   
   **Before (broken — 500 error, no diagnostic):**
   ```
   org.codehaus.groovy.runtime.InvokerInvocationException: ...
     Caused by: groovy.lang.MissingPropertyException: No such property: counter
   ```
   
   **After (clear compile-time error):**
   ```
   org.grails.taglib.GrailsTagException: JSP-style declaration blocks (<%! ... 
%>) are not supported
   in Groovy Server Pages. Use <% ... %> for scriptlet code or move logic to a 
controller or service.
   ```
   
   ## Root Cause
   
   The GSP parser (`GroovyPageParser.java`) uses a two-pass system:
   - **Pass 1** (`finalPass=false`): Pre-scan. Most handlers skip via `if 
(!finalPass) return;`
   - **Pass 2** (`finalPass=true`): Code generation. This is where the class 
body is written
   
   The `declare()` method uniquely has **inverted** logic — `if (finalPass) 
return;` — so it runs during Pass 1 only. This causes declaration content to be 
written to the output stream *before* the class definition, between the 
`package` statement and `class` declaration:
   
   ```groovy
   // Generated code (broken):
   package com.example.demo
   int counter = 0;           // ← written by declare() in Pass 1
   class DemoPage extends GroovyPage {
       void run() { ... }     // ← written in Pass 2
   }
   ```
   
   This creates an invalid Groovy script+class hybrid. Groovy compiles it 
without error, but at runtime the class body cannot see the script-level 
variable, causing `MissingPropertyException`.
   
   **This has been broken since the very first commit in Grails 1.1 (March 
2009).** The `declare()` method has never generated correct code. Zero 
real-world usage exists — searching all public GitHub repositories for `<%!` in 
`.gsp` files returns exactly 1 result (a Sublime Text syntax highlighting test 
file). The Groovy variant `!{ ... }!` has 0 results.
   
   ## Fix
   
   Replaced the broken `declare()` implementation with a `GrailsTagException` 
throw, following the same error pattern used throughout `GroovyPageParser.java`:
   
   ```java
   // Before (broken since Grails 1.1):
   private void declare(boolean gsp) {
       if (finalPass) {
           return;
       }
       out.println();
       write(scan.getToken().trim(), gsp);
       out.println();
       out.println();
   }
   
   // After (clear error):
   private void declare(boolean gsp) {
       String syntax = gsp ? "!{ ... }!" : "<%! ... %>";
       throw new GrailsTagException(
               "JSP-style declaration blocks (" + syntax + ") are not supported 
in Groovy Server Pages. " +
               "Use <% ... %> for scriptlet code or move logic to a controller 
or service.",
               pageName, getCurrentOutputLineNumber());
   }
   ```
   
   The error is thrown during parsing (before any code generation), so 
developers get immediate feedback with the exact file name, line number, and a 
suggestion for the correct alternative syntax.
   
   ## Files Changed
   
   | File | Change |
   |------|--------|
   | `grails-gsp/core/.../GroovyPageParser.java` | Changed `declare()` to throw 
`GrailsTagException` instead of writing broken code |
   | `grails-gsp/plugin/.../ParseSpec.groovy` | Added 3 Spock tests for 
declaration block rejection |
   
   ## Test Coverage
   
   3 new Spock tests added to `ParseSpec.groovy` (all 13 tests pass — 10 
existing + 3 new):
   
   1. **`parse with JSP declaration block throws error`** — verifies `<%! int 
counter = 0; %>` throws `GrailsTagException` with message containing 
"declaration blocks", "<%! ... %>", and "not supported"
   2. **`parse with JSP declaration block containing method throws error`** — 
verifies `<%! String hello() { return "hi"; } %>` embedded in HTML throws 
`GrailsTagException`
   3. **`parse with Groovy declaration block throws error`** — verifies `!{ int 
counter = 0; }!` (Groovy variant) throws `GrailsTagException` with message 
containing "!{ ... }!"
   
   ## Example Application
   
   **Repository:** 
https://github.com/jamesfredley/grails-gsp-declaration-block-bug
   
   Grails 7.0.7 app with two pages:
   
   1. `http://localhost:8080/bugDemo/safe` — uses `<% int counter = 0; %>` 
(scriptlet) — **renders correctly**
   2. `http://localhost:8080/bugDemo/index` — uses `<%! int counter = 0; %>` 
(declaration block) — **500 error** (demonstrates the bug)
   
   ```bash
   ./gradlew bootRun
   # Visit http://localhost:8080/bugDemo/safe   → works
   # Visit http://localhost:8080/bugDemo/index  → 500 error (bug)
   ```
   
   ## Environment Information
   
   - Grails: 7.0.7
   - GORM: 7.0.7
   - Spring Boot: 3.5.10
   - Groovy: 4.0.30
   - JDK: 17 (Amazon Corretto)
   
   ## Version
   
   7.0.7


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to