You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2008/09/16 23:12:38 UTC
svn commit: r696061 - in /tomcat/trunk/java/org/apache/jasper:
Constants.java compiler/Compiler.java compiler/Generator.java
compiler/JspDocumentParser.java compiler/JspUtil.java compiler/Parser.java
compiler/ParserController.java compiler/Validator.java
Author: markt
Date: Tue Sep 16 14:12:38 2008
New Revision: 696061
URL: http://svn.apache.org/viewvc?rev=696061&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=45451
Testing for this threw up all sorts of other failures around use of \${...} These should all now be fixed. The two pass parsing means we can do away with the previous 'replace with unused unicode character' trick.
Modified:
tomcat/trunk/java/org/apache/jasper/Constants.java
tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java
tomcat/trunk/java/org/apache/jasper/compiler/Generator.java
tomcat/trunk/java/org/apache/jasper/compiler/JspDocumentParser.java
tomcat/trunk/java/org/apache/jasper/compiler/JspUtil.java
tomcat/trunk/java/org/apache/jasper/compiler/Parser.java
tomcat/trunk/java/org/apache/jasper/compiler/ParserController.java
tomcat/trunk/java/org/apache/jasper/compiler/Validator.java
Modified: tomcat/trunk/java/org/apache/jasper/Constants.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/Constants.java?rev=696061&r1=696060&r2=696061&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/Constants.java (original)
+++ tomcat/trunk/java/org/apache/jasper/Constants.java Tue Sep 16 14:12:38 2008
@@ -181,13 +181,6 @@
System.getProperty("org.apache.jasper.Constants.TEMP_VARIABLE_NAME_PREFIX", "_jspx_temp");
/**
- * A replacement char for "\$".
- * XXX This is a hack to avoid changing EL interpreter to recognize "\$"
- */
- public static final char ESC = '\u001b';
- public static final String ESCStr = "'\\u001b'";
-
- /**
* Has security been turned on?
*/
public static final boolean IS_SECURITY_ENABLED =
Modified: tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java?rev=696061&r1=696060&r2=696061&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java (original)
+++ tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java Tue Sep 16 14:12:38 2008
@@ -145,8 +145,28 @@
ServletWriter writer = null;
try {
+ /*
+ * The setting of isELIgnored changes the behaviour of the parser
+ * in subtle ways. To add to the 'fun', isELIgnored can be set in
+ * any file that forms part of the translation unit so setting it
+ * in a file included towards the end of the translation unit can
+ * change how the parser should have behaved when parsing content
+ * up to the point where isELIgnored was set. Arghh!
+ * Previous attempts to hack around this have only provided partial
+ * solutions. We now use two passes to parse the translation unit.
+ * The first just parses the directives and the second parses the
+ * whole translation unit once we know how isELIgnored has been set.
+ * TODO There are some possible optimisations of this process.
+ */
// Parse the file
ParserController parserCtl = new ParserController(ctxt, this);
+
+ // Pass 1 - the directives
+ Node.Nodes directives =
+ parserCtl.parseDirectives(ctxt.getJspFile());
+ Validator.validateDirectives(this, directives);
+
+ // Pass 2 - the whole translation unit
pageNodes = parserCtl.parse(ctxt.getJspFile());
if (ctxt.isPrototypeMode()) {
@@ -158,8 +178,9 @@
return null;
}
- // Validate and process attributes
- Validator.validate(this, pageNodes);
+ // Validate and process attributes - don't re-validate the
+ // directives we validated in pass 1
+ Validator.validateExDirectives(this, pageNodes);
if (log.isDebugEnabled()) {
t2 = System.currentTimeMillis();
Modified: tomcat/trunk/java/org/apache/jasper/compiler/Generator.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/Generator.java?rev=696061&r1=696060&r2=696061&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/compiler/Generator.java (original)
+++ tomcat/trunk/java/org/apache/jasper/compiler/Generator.java Tue Sep 16 14:12:38 2008
@@ -806,13 +806,8 @@
}
return v;
} else if (attr.isELInterpreterInput()) {
- boolean replaceESC = v.indexOf(Constants.ESC) > 0;
v = JspUtil.interpreterCall(this.isTagFile, v, expectedType,
attr.getEL().getMapName(), false);
- // XXX ESC replacement hack
- if (replaceESC) {
- v = "(" + v + ").replace(" + Constants.ESCStr + ", '$')";
- }
if (encode) {
return "org.apache.jasper.runtime.JspRuntimeLibrary.URLEncode("
+ v + ", request.getCharacterEncoding())";
@@ -2839,16 +2834,10 @@
attrValue = sb.toString();
} else {
// run attrValue through the expression interpreter
- boolean replaceESC = attrValue.indexOf(Constants.ESC) > 0;
String mapName = (attr.getEL() != null) ? attr.getEL()
.getMapName() : null;
attrValue = JspUtil.interpreterCall(this.isTagFile,
attrValue, c[0], mapName, false);
- // XXX hack: Replace ESC with '$'
- if (replaceESC) {
- attrValue = "(" + attrValue + ").replace("
- + Constants.ESCStr + ", '$')";
- }
}
} else {
attrValue = convertString(c[0], attrValue, localName,
Modified: tomcat/trunk/java/org/apache/jasper/compiler/JspDocumentParser.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/JspDocumentParser.java?rev=696061&r1=696060&r2=696061&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/compiler/JspDocumentParser.java (original)
+++ tomcat/trunk/java/org/apache/jasper/compiler/JspDocumentParser.java Tue Sep 16 14:12:38 2008
@@ -580,6 +580,9 @@
lastCh = ch;
}
} else if (lastCh == '\\' && (ch == '$' || ch == '#')) {
+ if (pageInfo.isELIgnored()) {
+ ttext.write('\\');
+ }
ttext.write(ch);
ch = 0; // Not start of EL anymore
} else {
Modified: tomcat/trunk/java/org/apache/jasper/compiler/JspUtil.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/JspUtil.java?rev=696061&r1=696060&r2=696061&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/compiler/JspUtil.java (original)
+++ tomcat/trunk/java/org/apache/jasper/compiler/JspUtil.java Tue Sep 16 14:12:38 2008
@@ -177,7 +177,7 @@
returnString = expression;
}
- return escapeXml(returnString.replace(Constants.ESC, '$'));
+ return escapeXml(returnString);
}
/**
Modified: tomcat/trunk/java/org/apache/jasper/compiler/Parser.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/Parser.java?rev=696061&r1=696060&r2=696061&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/compiler/Parser.java (original)
+++ tomcat/trunk/java/org/apache/jasper/compiler/Parser.java Tue Sep 16 14:12:38 2008
@@ -266,6 +266,7 @@
private String parseQuoted(Mark start, String tx, char quote)
throws JasperException {
StringBuffer buf = new StringBuffer();
+ boolean possibleEL = tx.contains("${");
int size = tx.length();
int i = 0;
while (i < size) {
@@ -287,13 +288,22 @@
}
} else if (ch == '\\' && i + 1 < size) {
ch = tx.charAt(i + 1);
- if (ch == '\\' || ch == '\"' || ch == '\'' || ch == '>') {
+ if (ch == '\\' || ch == '\"' || ch == '\'') {
+ if (pageInfo.isELIgnored() || !possibleEL) {
+ // EL is not enabled or no chance of EL
+ // Unescape these now
+ buf.append(ch);
+ i += 2;
+ } else {
+ // EL is enabled and ${ appears in value
+ // EL processing will escape these
+ buf.append('\\');
+ buf.append(ch);
+ i += 2;
+ }
+ } else if (ch == '>') {
buf.append(ch);
i += 2;
- } else if (ch == '$') {
- // Replace "\$" with some special char. XXX hack!
- buf.append(Constants.ESC);
- i += 2;
} else {
buf.append('\\');
++i;
@@ -1339,11 +1349,8 @@
}
char next = (char) reader.peekChar();
// Looking for \% or \$ or \#
- // TODO: only recognize \$ or \# if isELIgnored is false, but since
- // it can be set in a page directive, it cannot be determined
- // here. Argh! (which is the way it should be since we shouldn't
- // convolude multiple steps at once and create confusing parsers...)
- if (next == '%' || next == '$' || next == '#') {
+ if (next == '%' || ((next == '$' || next == '#') &&
+ !pageInfo.isELIgnored())) {
ch = reader.nextChar();
}
}
Modified: tomcat/trunk/java/org/apache/jasper/compiler/ParserController.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/ParserController.java?rev=696061&r1=696060&r2=696061&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/compiler/ParserController.java (original)
+++ tomcat/trunk/java/org/apache/jasper/compiler/ParserController.java Tue Sep 16 14:12:38 2008
@@ -104,6 +104,24 @@
}
/**
+ * Parses the directives of a JSP page or tag file. This is invoked by the
+ * compiler.
+ *
+ * @param inFileName The path to the JSP page or tag file to be parsed.
+ */
+ public Node.Nodes parseDirectives(String inFileName)
+ throws FileNotFoundException, JasperException, IOException {
+ // If we're parsing a packaged tag file or a resource included by it
+ // (using an include directive), ctxt.getTagFileJar() returns the
+ // JAR file from which to read the tag file or included resource,
+ // respectively.
+ isTagFile = ctxt.isTagFile();
+ directiveOnly = true;
+ return doParse(inFileName, null, ctxt.getTagFileJarUrl());
+ }
+
+
+ /**
* Processes an include directive with the given path.
*
* @param inFileName The path to the resource to be included.
Modified: tomcat/trunk/java/org/apache/jasper/compiler/Validator.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/Validator.java?rev=696061&r1=696060&r2=696061&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/compiler/Validator.java (original)
+++ tomcat/trunk/java/org/apache/jasper/compiler/Validator.java Tue Sep 16 14:12:38 2008
@@ -1311,7 +1311,6 @@
}
} else {
- value = value.replace(Constants.ESC, '$');
result = new Node.JspAttribute(tai, qName, uri,
localName, value, false, null, dynamic);
}
@@ -1666,15 +1665,13 @@
}
}
- public static void validate(Compiler compiler, Node.Nodes page)
+ public static void validateDirectives(Compiler compiler, Node.Nodes page)
throws JasperException {
-
- /*
- * Visit the page/tag directives first, as they are global to the page
- * and are position independent.
- */
page.visit(new DirectiveVisitor(compiler));
+ }
+ public static void validateExDirectives(Compiler compiler, Node.Nodes page)
+ throws JasperException {
// Determine the default output content type
PageInfo pageInfo = compiler.getPageInfo();
String contentType = pageInfo.getContentType();
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org