Fixes PreserveKey not being passed from a redirect (#522)

[tonyjunkes]

Issue:

In Lucee, localmode="modern" is extremely strict with what scopes are referenced/available in the chain. Especially when a function within a function accepts a closure. This makes external local and variables scopes unavailable unless directly passed and returned for further use.

Fix:

The committed fix sets localmode="classic" in the function expression passed into sessionLock() inside of getNextPreserveKeyAndPurgeOld(). This allows FW/1's flash context to work as expected, regardless of the number of contexts, in both Lucee and Adobe ColdFusion (since the attribute is simply ignored).

Note On Alternative Approaches:

If it's preferred to avoid engine specific mechanics in the code, an alternative approach is to define both the "next" and "old" keys to session variables that are accessible to all levels of the execution chain in getNextPreserveKeyAndPurgeOld().

private string function getNextPreserveKeyAndPurgeOld() {
        try {
            sessionLock(function(){
                sessionDefault( '__fw1NextPreserveKey', 0 );
                sessionDefault( '__fw1OldKeyToPurge', '' );
                if ( variables.framework.maxNumContextsPreserved > 1 ) {
                    sessionWrite( '__fw1NextPreserveKey', sessionRead( '__fw1NextPreserveKey' ) + 1 );
                    sessionWrite( '__fw1OldKeyToPurge', sessionRead( '__fw1NextPreserveKey' ) - variables.framework.maxNumContextsPreserved );
                } else {
                    sessionWrite( '__fw1NextPreserveKey', '' );
                    sessionWrite( '__fw1PreserveKey', '' );
                }
            });
            var key = getPreserveKeySessionKey( sessionRead( '__fw1OldKeyToPurge' ) );
            if ( sessionHas( key ) ) {
                sessionDelete( key );
            }
        } catch ( any e ) {
            // ignore - assume session scope is disabled
        }
        return sessionRead( '__fw1NextPreserveKey' );
}

[aliaspooryorik]

Would Lucee let you call a function to access the variables scope? Something like:

private numeric function getMaxNumContextsPreserved() {
  return variables.framework.maxNumContextsPreserved;
}

and then call it like so:

sessionLock(function(){
    var maxNumContextsPreserved = getMaxNumContextsPreserved();
    if ( maxNumContextsPreserved > 1 ) {
        sessionDefault( '__fw1NextPreserveKey', 1 );
        nextPreserveKey = sessionRead( '__fw1NextPreserveKey' );
        sessionWrite( '__fw1NextPreserveKey', nextPreserveKey + 1 );
        oldKeyToPurge = nextPreserveKey - maxNumContextsPreserved;
    } else {
        nextPreserveKey = '';
        sessionWrite( '__fw1PreserveKey', nextPreserveKey );
        oldKeyToPurge = '';
    }
});

[tonyjunkes]

Yes, you should be able to access the value in that manner. I'm not sure I follow what your question may suggest?

The issue isn't around that value specifically as it's actually the local declarations of nextPreserveKey and oldKeyToPurge from the caller not being referenced/updated inside the function expression passed to sessionLock(). This results in the return value of the calling function being empty every time. (Unless maxNumContextsPreserved=1)

localmode=modern seems to be so strict that it only checks up the chain if you are referencing a variable in a parent scope. Any declarations that would normally update an existing assignment in the scope chain are completely omitted and an assignment is made inside the immediate scope instead. At least that is my assumption/findings from testing. I personally haven't spent much time with this feature in Lucee so its functionality is... mysterious.

[aliaspooryorik]

Ah OK, sorry, got the wrong end of the stick, so ignore me :)

localmode=modern does sound like it is overly strict in this situation.

[sneiland]

Im of the opinion that the suggested fix is acceptable in its current form for now, if ACF introduces a similar scoping issue later on we can revisit alternatives later. Once I get the current beta released I'll merge this in for the next release.

[tonyjunkes]

I think this closed because I reset my repos develop branch to work on another issue. I’ll create a new branch on my end and recommit I guess.