Skip to content

Commit 6c8aede

Browse files
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 69de23f commit 6c8aede

File tree

4 files changed

+83
-9
lines changed

4 files changed

+83
-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 static final int EXPR_MAYBE = 1;
7474

75+
/** Expression state constant */
76+
private static final int EXPR_INSIDE_STRING = 2;
77+
78+
/** Expression state constant */
79+
private static final int EXPR_INSIDE_STRING_AFTER_BACKSLASH = 3;
80+
7581
static final Set<String> VOID_ELEMENTS = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(
7682
"area", "base", "br", "col", "embed", "hr", "img", "input", "link", "meta", "param", "source", "track",
7783
"wbr")));
@@ -403,17 +409,35 @@ private void update(final char[] buf, int len) throws IOException {
403409
break;
404410

405411
case EXPRESSION:
406-
if (exprType == EXPR_MAYBE && c != '{') {
407-
// not a valid expression
408-
if (c == '<') {
409-
// reset to process tag correctly
410-
curr--;
412+
if (exprType == EXPR_MAYBE) {
413+
if (c == '{') {
414+
exprType = EXPR_NONE;
415+
} else {
416+
// not a valid expression
417+
if (c == '<') {
418+
// reset to process tag correctly
419+
curr--;
420+
}
421+
parseState = PARSE_STATE.OUTSIDE;
422+
}
423+
} else if (exprType == EXPR_NONE) {
424+
if (c == '}') {
425+
parseState = PARSE_STATE.OUTSIDE;
426+
} else if (c == '"' || c == '\'') {
427+
exprType = EXPR_INSIDE_STRING;
428+
quoteChar = c;
429+
}
430+
} else if (exprType == EXPR_INSIDE_STRING) {
431+
if (c == '\\') {
432+
exprType = EXPR_INSIDE_STRING_AFTER_BACKSLASH;
433+
} else if (c == quoteChar) {
434+
exprType = EXPR_NONE;
411435
}
412-
parseState = PARSE_STATE.OUTSIDE;
413-
} else if (c == '}') {
414-
parseState = PARSE_STATE.OUTSIDE;
436+
} else {
437+
assert exprType == EXPR_INSIDE_STRING_AFTER_BACKSLASH;
438+
// ignore whatever came after the backslash and stay inside the string
439+
exprType = EXPR_INSIDE_STRING;
415440
}
416-
exprType = EXPR_NONE;
417441
break;
418442
default:
419443
break;

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,14 @@ public void testUriManipulationDataSlyUse() {
238238
assertTrue(secondArgument instanceof MapLiteral);
239239
}
240240

241+
@Test
242+
public void testMarkupInStringLiteral() {
243+
CompilationResult result = compileFile("/markup-in-string-literal.html");
244+
assertTrue(
245+
"Didn't expect any warnings or errors.",
246+
result.getErrors().isEmpty() && result.getWarnings().isEmpty());
247+
}
248+
241249
private CompilationResult compileFile(final String file) {
242250
InputStream stream = this.getClass().getResourceAsStream(file);
243251
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
@@ -244,6 +244,29 @@ public void testExplicitlyClosedElements() throws IOException {
244244
handler.onStartElementInvocations);
245245
}
246246

247+
@Test
248+
public void testTrickyStringLiterals() throws IOException {
249+
String[] testStrings = new String[] {
250+
"${'easy case'}",
251+
"${'looks like <html> but isn\\'t}",
252+
"${'not over yet } but now it's over'}",
253+
"${'not over } <br> now it's over'}",
254+
"${'brace } <html> } escaped apos \\' } done'}",
255+
"${\"brace } <html> } escaped quote \\\" } done\"}"
256+
};
257+
258+
for (String testString : testStrings) {
259+
Template reference = new Template();
260+
reference.addChild(new TemplateTextNode(testString));
261+
262+
TemplateParser.TemplateParserContext context = new TemplateParser.TemplateParserContext();
263+
HtmlParser.parse(new StringReader(testString), context);
264+
Template parsed = context.getTemplate();
265+
266+
assertSameStructure(reference, parsed);
267+
}
268+
}
269+
247270
abstract class AbstractDocumentHandler implements DocumentHandler {
248271

249272
private String lastProcessedTag;
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)