Skip to content

Commit 48eadc5

Browse files
csabokacsaboka
committed
SLING-12325 - Improve parsing of tricky expressions
To detect the end of the expression properly, it isn't enough to look for a closing brace, we also need to ensure that the brace is outside of a string literal.
1 parent 19acf85 commit 48eadc5

File tree

4 files changed

+81
-9
lines changed

4 files changed

+81
-9
lines changed

‎src/main/java/org/apache/sling/scripting/sightly/impl/html/dom/HtmlParser.java‎

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,12 @@ private enum PARSE_STATE {
7272
/** Expression state constant */
7373
private final static int EXPR_MAYBE = 1;
7474

75+
/** Expression state constant */
76+
private final static int EXPR_INSIDE_STRING = 2;
77+
78+
/** Expression state constant */
79+
private final static int EXPR_INSIDE_STRING_AFTER_BACKSLASH = 3;
80+
7581
final static Set<String> VOID_ELEMENTS = Collections.unmodifiableSet(new HashSet<>(
7682
Arrays.asList("area", "base", "br", "col", "embed", "hr", "img", "input", "link", "meta", "param", "source", "track", "wbr")));
7783

@@ -398,17 +404,35 @@ private void update(final char[] buf, int len) throws IOException {
398404
break;
399405

400406
case EXPRESSION:
401-
if (exprType == EXPR_MAYBE && c != '{') {
402-
// not a valid expression
403-
if (c == '<') {
404-
//reset to process tag correctly
405-
curr--;
407+
if (exprType == EXPR_MAYBE) {
408+
if (c == '{') {
409+
exprType = EXPR_NONE;
410+
} else {
411+
// not a valid expression
412+
if (c == '<') {
413+
//reset to process tag correctly
414+
curr--;
415+
}
416+
parseState = PARSE_STATE.OUTSIDE;
417+
}
418+
} else if (exprType == EXPR_NONE) {
419+
if (c == '}') {
420+
parseState = PARSE_STATE.OUTSIDE;
421+
} else if (c == '"' || c == '\'') {
422+
exprType = EXPR_INSIDE_STRING;
423+
quoteChar = c;
424+
}
425+
} else if (exprType == EXPR_INSIDE_STRING) {
426+
if (c == '\\') {
427+
exprType = EXPR_INSIDE_STRING_AFTER_BACKSLASH;
428+
} else if (c == quoteChar) {
429+
exprType = EXPR_NONE;
406430
}
407-
parseState = PARSE_STATE.OUTSIDE;
408-
} else if (c == '}') {
409-
parseState = PARSE_STATE.OUTSIDE;
431+
} else {
432+
assert exprType == EXPR_INSIDE_STRING_AFTER_BACKSLASH;
433+
// ignore whatever came after the backslash and stay inside the string
434+
exprType = EXPR_INSIDE_STRING;
410435
}
411-
exprType = EXPR_NONE;
412436
break;
413437
default:
414438
break;

‎src/test/java/org/apache/sling/scripting/sightly/impl/compiler/SightlyCompilerTest.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,12 @@ public void testUriManipulationDataSlyUse() {
208208
assertTrue(secondArgument instanceof MapLiteral);
209209
}
210210

211+
@Test
212+
public void testMarkupInStringLiteral() {
213+
CompilationResult result = compileFile("/markup-in-string-literal.html");
214+
assertTrue("Didn't expect any warnings or errors.", result.getErrors().isEmpty() && result.getWarnings().isEmpty());
215+
}
216+
211217
private CompilationResult compileFile(final String file) {
212218
InputStream stream = this.getClass().getResourceAsStream(file);
213219
final Reader reader = new InputStreamReader(stream);

‎src/test/java/org/apache/sling/scripting/sightly/impl/html/dom/HtmlParserTest.java‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,29 @@ public void testExplicitlyClosedElements() throws IOException {
232232
handler.onStartElementInvocations);
233233
}
234234

235+
@Test
236+
public void testTrickyStringLiterals() throws IOException {
237+
String[] testStrings = new String[] {
238+
"${'easy case'}",
239+
"${'looks like <html> but isn\\'t}",
240+
"${'not over yet } but now it's over'}",
241+
"${'not over } <br> now it's over'}",
242+
"${'brace } <html> } escaped apos \\' } done'}",
243+
"${\"brace } <html> } escaped quote \\\" } done\"}"
244+
};
245+
246+
for (String testString : testStrings) {
247+
Template reference = new Template();
248+
reference.addChild(new TemplateTextNode(testString));
249+
250+
TemplateParser.TemplateParserContext context = new TemplateParser.TemplateParserContext();
251+
HtmlParser.parse(new StringReader(testString), context);
252+
Template parsed = context.getTemplate();
253+
254+
assertSameStructure(reference, parsed);
255+
}
256+
}
257+
235258
abstract class AbstractDocumentHandler implements DocumentHandler {
236259

237260
private String lastProcessedTag;

‎src/test/resources/markup-in-string-literal.html‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<!--~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2+
~ Licensed to the Apache Software Foundation (ASF) under one
3+
~ or more contributor license agreements. See the NOTICE file
4+
~ distributed with this work for additional information
5+
~ regarding copyright ownership. The ASF licenses this file
6+
~ to you under the Apache License, Version 2.0 (the
7+
~ "License"); you may not use this file except in compliance
8+
~ with the License. You may obtain a copy of the License at
9+
~
10+
~ http://www.apache.org/licenses/LICENSE-2.0
11+
~
12+
~ Unless required by applicable law or agreed to in writing,
13+
~ software distributed under the License is distributed on an
14+
~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
~ KIND, either express or implied. See the License for the
16+
~ specific language governing permissions and limitations
17+
~ under the License.
18+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~-->
19+
${'Click <a href="{0}">here</a> to learn more' @ format='https://example.com/'}

0 commit comments

Comments
 (0)