Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarification required for declaration of scripting variables for actions #42

Closed
glassfishrobot opened this issue May 15, 2014 · 4 comments

Comments

@glassfishrobot
Copy link

glassfishrobot commented May 15, 2014

There are some grey areas in the JSP specification regarding when and how scripting variables associated with actions are declared that need to be clarified.

Consider the following from JSP.9.4.4

<x:tag>  
  foo
</x:tag>

Equivalent Text:

declare AT_BEGIN variables
{
  declare NESTED variables
  transformation of foo
}
declare AT_END variables

If <x:tag> declares an AT_BEGIN scripting variable myVar of type String then the equivalent text becomes (ignoring the required variable synchronization to make the example clearer)

String myVar = null;
{
  transformation of foo
}

If our original code is extended to:

<x:tag>  
  foo
</x:tag>
<x:tag>  
  foo
</x:tag>

then the equivalent text becomes

String myVar = null;
{
  transformation of foo
}
String myVar = null;
{
  transformation of foo
}

Clearly this is problematic since it is illegal to declare the same variable multiple times in the same scope.

There are a couple of ways to handle this. The first is to insert an outer code block around the code for each tag. However, that makes little sense since it makes the declaration of AT_END variables pointless as the following example shows. There is no point declaring the AT_END variables in this case since they instantly go out of scope.

{
  declare AT_BEGIN variables
  {
    declare NESTED variables
    transformation of foo
  }
  declare AT_END variables
}
{
  declare AT_BEGIN variables
  {
    declare NESTED variables
    transformation of foo
  }
  declare AT_END variables
}

Another alternative is to pre-process the tags and to ensure that duplicate scripting variables are not defined. The equivalent text for multiple tag instances then becomes:

String myVar = null;
{
  transformation of foo
}
{
  transformation of foo
}

The pre-processing approach works in most cases but it can break if scriptlets are present. Consider the following original text:

<% try { %>
  <x:tag>  
    foo
  </x:tag>
<% } catch (IllegalArgumentException iae) { %>
  <x:tag>  
    foo
  </x:tag>
<% } %>

With pre-processing this becomes:

try {
  String myVar = null;
  {
    transformation of foo
  }
} catch (IllegalArgumentException iae) {
  {
    transformation of foo
  }
}

which will break if the transformation of foo refers to myVar (which it almost certainly will) because myVar is now out of scope for the second instance of the tag.

I do not believe it is reasonable or practical for JSP containers to parse scriplets surrounding actions to determine the scope of any declared scripting variables and then adjust the declaration of those scripting variables accordingly.

One potential solution is to move the declaration of all scripting variables for actions (AT_BEGIN, NESTED and AT_END) to immediately after the declaration of the implicit scripting variables so the scripting variables for actions are always declared once only and are always in scope during the execution of the page.

It is necessary for the JSP specification to clarify how scripting variables declared by actions should be handled. It should be noted that there are variations on this problem such as when tags are nested.

For background on the Tomcat bug report that prompted this issue see:
https://issues.apache.org/bugzilla/show_bug.cgi?id=56516
Also see:
https://issues.apache.org/bugzilla/show_bug.cgi?id=42390
https://issues.apache.org/bugzilla/show_bug.cgi?id=48616

@glassfishrobot
Copy link
Author

@glassfishrobot Commented
Reported by markt_asf

@glassfishrobot
Copy link
Author

@glassfishrobot Commented
This issue was imported from java.net JIRA JSP-42

@glassfishrobot
Copy link
Author

@markt-asf
Copy link
Contributor

I have been spending some time looking at this issue.

The existing specification and Javadoc appear, to me at least, to make conflicting statements relating to exactly where scripting variables defined by tags should be declared.

JSP.9.4.4 defines transformations that include when scripting variables should be declared but JSP.9.1.3 states that the transformations do not have to be performed literally.
JSP.9.4.4 states that scope does not affect visibility of the scripting variables within the generated program but the Javadoc for the scope constants in VariableInfo state that the chosen scope does impact visibility.
JSP.9.4.4 states that visibility is affected by other constructs such as scriptlets.

An additional factor is that previous attempts to make changes in this area has triggered regressions and I'd like to avoid that when resolving this issue, particularly since there is relatively little time available between now and the planned completion date for JSP 3.1 for implementations to make changes, test and provide feedback.

My own testing suggests that moving scripting variable definition to the declarations section will address this issue in most cases but that there are scenarios where such a change will break an existing page.

Given:

  • that this issue appears to only have affected a small number of users
  • that this issue has existed for most of the lifetime of the specification
  • the maturity of the specification

rather than introduce a change, or even a configuration option, my current intention is to:

  • clarify the existing specification text and Javadoc to remove the conflicting statements
  • clarify that "scope" in this instance defines when the scripting variable is synchronised with the page scoped attribute and does not mean scope as defined by the scripting language
  • note that containers may wish to provide container specific options for alternative approaches to declarations
  • note that users have full flexibility by setting declare=false for the scripting variables and explicitly declaring them as required

Unless there are objections, I hope to have a PR along the above lines ready for review towards the end of this week.

markt-asf added a commit to markt-asf/jakartaee-pages that referenced this issue Oct 1, 2021
markt-asf added a commit to markt-asf/jakartaee-pages that referenced this issue Oct 1, 2021
markt-asf added a commit to markt-asf/jakartaee-pages that referenced this issue Oct 1, 2021
markt-asf added a commit that referenced this issue Oct 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants