markt-asf commented on code in PR #842:
URL: https://github.com/apache/tomcat/pull/842#discussion_r2045039959
##########
java/org/apache/jasper/runtime/JspRuntimeLibrary.java:
##########
@@ -957,4 +957,21 @@ public static void releaseTag(Tag tag, InstanceManager
instanceManager) {
}
}
+
+ public static void nonstandardSetTag(jakarta.servlet.jsp.PageContext
pageContext, String var, Object value) {
+ if (value == null) {
+ pageContext.removeAttribute(var);
+ } else {
+ pageContext.setAttribute(var, value);
+ }
+ }
+
Review Comment:
This only appears to be used in test code. If true, it should be removed.
##########
java/org/apache/jasper/compiler/Generator.java:
##########
@@ -3028,6 +3036,195 @@ public String
generateNamedAttributeJspFragment(Node.NamedAttribute n, String ta
return varName;
}
+
+ /**
+ * Determines whether a tag should be handled via nonstandard code
(typically
+ * faster). Considers both configuration and level of support within
Tomcat.
+ *
+ * Note that Tomcat is free to ignore any case it cannot handle, as
long as it
+ * reports it accurately to the caller by returning false. For
example, the
+ * initial implementation for c:set excludes support for body content.
c:set
+ * tags with body content will be generated with the standard code and
tags
+ * without body content will be generated via non-standard code.
+ *
+ * @param n
+ * @param jspAttributes
+ * @return whether code was generated
+ * @throws JasperException
+ */
+ private boolean visitPotentiallyNonstandardCustomTag(Node.CustomTag n)
+ throws JasperException {
+ if (!nonstandardCustomTagNames.contains(n.getQName())) {
+ // tag is not configured, move along
+ return false;
+ }
+
+ Map<String, JspAttribute> jspAttributes = new HashMap<>();
+ if (n.getJspAttributes() != null) {
+ for (JspAttribute jspAttr : n.getJspAttributes()) {
+ jspAttributes.put(jspAttr.getLocalName(), jspAttr);
+ }
+ }
+ switch (n.qName) {
+ case "c:set":
+ // requires var and value, scope is optional, body is
prohibited, value cannot be deferred
+ if (n.hasEmptyBody()
+ && jspAttributes.containsKey("var")
+ && jspAttributes.containsKey("value")
+ && CORE_LIBS_URI.equals(n.getURI())) {
+ // verify value is not a deferred expression
+ String valueText = jspAttributes.get("value").getValue();
+ if (valueText.startsWith("#")) {
+ return false;
+ } else if (jspAttributes.size() == 2
+ || (jspAttributes.size() == 3 &&
jspAttributes.containsKey("scope"))) {
+ generateNonstandardSetLogic(n, jspAttributes);
+ return true;
+ }
+ }
+ break;
+ case "c:remove":
+ // requires var, scope is optional, body is prohibited
+ if (n.hasEmptyBody()
+ && jspAttributes.containsKey("var")
+ && CORE_LIBS_URI.equals(n.getURI())
+ && (jspAttributes.size() == 1
+ || (jspAttributes.size() == 2
+ && jspAttributes.containsKey("scope")))) {
+ generateNonstandardRemoveLogic(n, jspAttributes);
+ return true;
+
+ }
+ break;
+ default:
+ // This indicates someone configured a tag with no
non-standard implementation.
+ // Harmless.
+ }
+ return false;
+ }
+
+ private void generateNonstandardSetLogic(Node.CustomTag n, Map<String,
JspAttribute> jspAttributes)
+ throws JasperException {
+ String baseVar = createTagVarName(n.getQName(), n.getPrefix(),
n.getLocalName());
+ String tagMethod = "_jspx_meth_" + baseVar;
+ ServletWriter outSave = out;
+
+ // generate method call
+ out.printin(tagMethod);
+ out.print("(");
+ out.print("_jspx_page_context");
+ out.println(");");
+ GenBuffer genBuffer = new GenBuffer(n, n.getBody());
+ methodsBuffered.add(genBuffer);
+ out = genBuffer.getOut();
+
+ // Generate code for method declaration
+ methodNesting++;
+ out.println();
+ out.pushIndent();
+ out.printin("private void ");
+ out.print(tagMethod);
+ out.println("(jakarta.servlet.jsp.PageContext
_jspx_page_context)");
+ out.printil(" throws java.lang.Throwable {");
+ out.pushIndent();
+ // Generated body of method
+ out.printil("// " + n.getQName());
+
+ JspAttribute varAttribute = jspAttributes.get("var");
+ Mark m = n.getStart();
+ out.printil("// " + m.getFile() + "(" + m.getLineNumber() + "," +
m.getColumnNumber() + ") "
+ + varAttribute.getTagAttributeInfo());
+
+ JspAttribute valueAttribute = jspAttributes.get("value");
+ m = n.getStart();
+ out.printil("// " + m.getFile() + "(" + m.getLineNumber() + "," +
m.getColumnNumber() + ") "
+ + valueAttribute.getTagAttributeInfo());
+
+ String varValue = varAttribute.getValue();
+ String scopeValue = translateScopeToConstant(jspAttributes);
+ String evaluatedAttribute =
evaluateAttribute(getTagHandlerInfo(n), valueAttribute,
+ n, null);
+
out.printil("org.apache.jasper.runtime.JspRuntimeLibrary.nonstandardSetTag(_jspx_page_context,
\""
+ + varValue + "\", " + evaluatedAttribute + ", " +
scopeValue + ");");
+ // Generate end of method
+ out.popIndent();
+ out.printil("}");
+ out.popIndent();
+
+ methodNesting--;
+ // restore previous writer
+ out = outSave;
+ }
+
+ private String translateScopeToConstant(Map<String, JspAttribute>
jspAttributes) {
+ String scopeValue;
+ JspAttribute scopeAttribute = jspAttributes.get("scope");
+ if (scopeAttribute == null) {
+ scopeValue = "jakarta.servlet.jsp.PageContext.PAGE_SCOPE";
+ } else {
+ switch (scopeAttribute.getValue()) {
+ case "":
+ case "page":
+ scopeValue = "jakarta.servlet.jsp.PageContext.PAGE_SCOPE";
+ break;
+ case "request":
+ scopeValue =
"jakarta.servlet.jsp.PageContext.REQUEST_SCOPE";
+ break;
+ case "session":
+ scopeValue =
"jakarta.servlet.jsp.PageContext.SESSION_SCOPE";
+ break;
+ case "application":
+ scopeValue =
"jakarta.servlet.jsp.PageContext.APPLICATION_SCOPE";
+ break;
+ default:
+ throw new
IllegalArgumentException(Localizer.getMessage("jsp.error.page.invalid.scope"));
+ }
+ }
+ return scopeValue;
+ }
+
+ private void generateNonstandardRemoveLogic(Node.CustomTag n,
Map<String, JspAttribute> jspAttributes)
+ throws JasperException {
+ String baseVar = createTagVarName(n.getQName(), n.getPrefix(),
n.getLocalName());
+ String tagMethod = "_jspx_meth_" + baseVar;
+ ServletWriter outSave = out;
+
+ // generate method call
+ out.printin(tagMethod);
+ out.print("(");
+ out.print("_jspx_page_context");
+ out.println(");");
+ GenBuffer genBuffer = new GenBuffer(n, n.getBody());
+ methodsBuffered.add(genBuffer);
+ out = genBuffer.getOut();
+
+ // Generate code for method declaration
+ methodNesting++;
+ out.println();
+ out.pushIndent();
+ out.printin("private void ");
+ out.print(tagMethod);
+ out.println("(jakarta.servlet.jsp.PageContext pageContext)");
+ out.printil(" throws java.lang.Throwable {");
+ out.pushIndent();
+ // Generated body of method
+ String varValue = jspAttributes.get("var").getValue();
+ JspAttribute scope = jspAttributes.get("scope");
+ if (scope == null) {
+ out.printil("pageContext.removeAttribute(\"" + varValue +
"\");");
+ } else {
+ String scopeValue = translateScopeToConstant(jspAttributes);
+ out.printil("pageContext.removeAttribute(\"" + varValue + "\",
" + scopeValue + ");");
+ }
Review Comment:
This takes a different approach to handling `null` scope compared to `set`.
It isn't wrong but it is inconsistent. Why? If there is a good reason, it
should be documented else folks coming later are going to wonder why. If there
isn't a good reason, `set` and `remove` should be consistent (and I have no
preference for either approach).
--
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: dev-unsubscr...@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
