grails / grails-core

The Grails Web Application Framework
http://grails.org
Apache License 2.0
2.78k stars 951 forks source link

flashScope does not persist a redirect #13141

Closed cdaoust closed 11 months ago

cdaoust commented 11 months ago

Expected Behavior

flashScope should persist one redirect

Actual Behaviour

action redirected to has all new flashScope

Steps To Reproduce

Environment Information

Example Application

No response

Version

5.32

osscontributor commented 11 months ago

See the project at github.com/jeffbrown/issue13141.

grails-app/controllers/issue13141/DemoController.groovy

package issue13141

class DemoController {

    def first() {
        flash.genre = 'Jam Band'
        redirect action: 'second'
    }

    def second() {
        render "Genre: ${flash.genre}"
    }
}

grails-app/controllers/issue13141/UrlMappings.groovy#L12

package issue13141

class UrlMappings {

    static mappings = {
        "/$controller/$action?/$id?(.$format)?"{
            constraints {
                // apply constraints here
            }
        }

        "/doit"(controller: 'demo', action: 'first')

        "/"(view:"/index")
        "500"(view:'/error')
        "404"(view:'/notFound')
    }
}

If I run that and send a browser request to http://localhost:8080/doit the response body is "Genre: Jam Band" indicating that the flash did survive the redirect.

Is your project doing something fundamentally different than that?

osscontributor commented 11 months ago

@cdaoust Are you initiating your request from a browser or some other tool that has the notion of an http session?

cdaoust commented 11 months ago

Yes were using tomcat and flashScope on redirects did work previously. I should have tried a stripped down version to see it wasnt the new version of grails. After some thought, and I should have thought of this sooner, but we switched to spring jdbc sessions as we moved our apps to containers. Does the use of spring jdbc sessions preclude the correct working of flashScope perhaps? I'll try removing jdbc sessions to see if that is the culprit. Although that wont be a fix - we want to continue using jdbc sessions.

cdaoust commented 11 months ago

The issue is spring jdbc session. Any ideas on another way to maintain flashScope since the it seems to be reinitialising flashScope on every request. Is it because spring knows nothing about flash - strictly grails level.

cdaoust commented 11 months ago

OK after finally realizing the problem I did find a solution. Hopefully this helps someone else. @jeffbrown Thanks for looking into it and sorry to have wasted your time.

package test

import javax.servlet.http.HttpSession

class FlashInterceptor { FlashInterceptor() { matchAll().excludes(controller:"login") }

// lifted from obsolete Grails 3 plugin:
// https://github.com/jeetmp3/spring-session/blob/master/src/main/java/org/grails/plugins/springsession/web/http/HttpSessionSynchronizer.java
// synchronize the http session to make flash scope messages work correctly when using jdbc session store
boolean before() {
    if (request != null && request.getSession(false) != null) {
        HttpSession session = request.getSession()
        Enumeration<String> attributeNames = session.getAttributeNames()
        while (attributeNames.hasMoreElements()) {
            String key = attributeNames.nextElement()
            try {
                Object object = session.getAttribute(key)
                session.setAttribute(key, object)
            } catch (Exception ignored) {
            }
        }
    }
    true
}

boolean after() {
    true
}

void afterView() {
    // no-op
}

}

osscontributor commented 11 months ago
    if (request != null && request.getSession(false) != null) {
        HttpSession session = request.getSession()
        Enumeration<String> attributeNames = session.getAttributeNames()
        while (attributeNames.hasMoreElements()) {
            String key = attributeNames.nextElement()
            try {
                Object object = session.getAttribute(key)
                session.setAttribute(key, object)
            } catch (Exception ignored) {
            }
        }
    }

@cdaoust If you remove the if (request != null && request.getSession(false) != null) { conditional stuff and delete the line HttpSession session = request.getSession() so you are referencing the session property provided by the Interceptor trait, do you see the same results you see using the code you show there?

cdaoust commented 11 months ago

Yes that works - nice cleanup! thanks.

cdaoust commented 11 months ago

HI again Jeff,

As it is the flash scope will ALWAYS persist twice, redirect or not. This is not the objective. We only want to keep it on a redirect for second request. So now if I have a page that displays something from flash, the next page will also display the same - even if not redirected. Any ideas on how to solve this puzzle? How does Grails handle flash to persist only on redirects?

cdaoust commented 11 months ago

Heres' what I got working finally in case anyone comes looking here again:

boolean before() {
    try {
        def flashRedirect = session.getAttribute('request-redirect')
        if (flashRedirect) {
            session.setAttribute('request-redirect', false)
            def flashScope = session.getAttribute('org.grails.FLASH_SCOPE')
            session.setAttribute('org.grails.FLASH_SCOPE', flashScope)
        }
    } catch (Exception ignored) {}
    true
}

boolean after() {
    int status = response.status
    if (status == 302) {
        session.setAttribute('request-redirect', true)
        def flashScope = session.getAttribute('org.grails.FLASH_SCOPE')
        session.setAttribute('org.grails.FLASH_SCOPE', flashScope)
    }
    true
}
osscontributor commented 11 months ago

How does Grails handle flash to persist only on redirects?

I don't think it does. I think when you put something in flash scope, that information is intended be there for the remainder of the request which put it there, and 1 subsequent request. If that subsequent request comes from a redirect, or if it doesn't, I think flash is intended to behave that way either way.