Skip to content

Commit f140841

Browse files
committed
GROOVY-12269: Escape interpolation and line terminators in getOutputStatement
ScriptEngineFactory.getOutputStatement takes the text to display, which is data, and Groovy renders it as a double-quoted literal escaping only the quote and the backslash. Two characters were left to reach the generated code unaltered: - a dollar makes the literal an interpolating GString, so display text containing ${...} is evaluated rather than printed, and a bare $name leaks a binding; - a line terminator ends the line, so display text spanning lines emits a statement that does not compile at all. Escape both, along with the carriage return. Other control characters such as tab are legal inside a Groovy string literal and are left alone, so output is unchanged for every input that already worked. Nashorn is the precedent here: its factory had the same class of defect, reported on nashorn-dev in January 2017 as producing a syntax error for display text containing quotes, and was fixed by quoting the argument rather than by reinterpreting it as code. Groovy had already chosen the same reading by quoting and escaping at all; this completes it. getProgram and getMethodCallSyntax take code by contract and are unchanged. The factory's code-generating methods had no tests. The new ones assert the emitted spelling for each escaped character, and separately evaluate the generated statement and compare what it prints with the original text, since displaying the text verbatim is the actual contract. Without the fix the first fails on the dollar and the second fails to compile.
1 parent e430766 commit f140841

2 files changed

Lines changed: 111 additions & 1 deletion

File tree

subprojects/groovy-jsr223/src/main/java/org/codehaus/groovy/jsr223/GroovyScriptEngineFactory.java

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,9 +209,16 @@ public String getMethodCallSyntax(String obj, String method,
209209

210210
/**
211211
* Produces a Groovy statement that prints the supplied text.
212+
* <p>
213+
* The text is data, so it is rendered as a string literal with every character escaped
214+
* that would otherwise change what the statement means: the quote and backslash that
215+
* would end or re-escape the literal, the dollar that would make it an interpolating
216+
* {@link groovy.lang.GString}, and the line terminators that would end the line. Other
217+
* control characters are legal inside a Groovy string literal and are emitted as they
218+
* are.
212219
*
213220
* @param toDisplay the text to render
214-
* @return a {@code println} statement with embedded quotes and backslashes escaped
221+
* @return a {@code println} statement which displays the text verbatim
215222
*/
216223
@Override
217224
public String getOutputStatement(String toDisplay) {
@@ -227,6 +234,15 @@ public String getOutputStatement(String toDisplay) {
227234
case '\\':
228235
buf.append("\\\\");
229236
break;
237+
case '$':
238+
buf.append("\\$");
239+
break;
240+
case '\n':
241+
buf.append("\\n");
242+
break;
243+
case '\r':
244+
buf.append("\\r");
245+
break;
230246
default:
231247
buf.append(ch);
232248
break;
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
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+
package org.codehaus.groovy.jsr223
20+
21+
import org.junit.jupiter.api.Test
22+
23+
import javax.script.ScriptEngine
24+
import javax.script.ScriptEngineFactory
25+
26+
/**
27+
* Tests the code-generating methods of {@link GroovyScriptEngineFactory}.
28+
*/
29+
final class GroovyScriptEngineFactoryTest {
30+
31+
private final ScriptEngineFactory factory = new GroovyScriptEngineFactory()
32+
33+
/** Runs a generated statement and returns what it printed. */
34+
private String outputOf(String statement) {
35+
ScriptEngine engine = factory.scriptEngine
36+
StringWriter captured = new StringWriter()
37+
engine.context.writer = captured
38+
engine.eval(statement)
39+
captured.toString()
40+
}
41+
42+
@Test
43+
void testOutputStatementOfPlainText() {
44+
assert factory.getOutputStatement('context') == 'println("context")'
45+
}
46+
47+
@Test
48+
void testOutputStatementEscapesCharactersThatWouldChangeItsMeaning() {
49+
// A quote or backslash ends or re-escapes the literal, a dollar turns it into an
50+
// interpolating GString, and a line terminator ends the line.
51+
assert factory.getOutputStatement('"') == 'println("\\"")'
52+
assert factory.getOutputStatement('\\') == 'println("\\\\")'
53+
assert factory.getOutputStatement('$') == 'println("\\$")'
54+
assert factory.getOutputStatement('\n') == 'println("\\n")'
55+
assert factory.getOutputStatement('\r') == 'println("\\r")'
56+
}
57+
58+
@Test
59+
void testOutputStatementLeavesHarmlessControlCharactersAlone() {
60+
// Legal inside a Groovy string literal, so escaping them would change the emitted
61+
// text for input that already worked.
62+
assert factory.getOutputStatement('a\tb') == 'println("a\tb")'
63+
}
64+
65+
@Test
66+
void testGeneratedStatementDisplaysTheTextVerbatim() {
67+
// The contract is what the statement displays, not how it is spelled, so evaluate it.
68+
['context',
69+
'plain text',
70+
'quotes " and \\ backslashes',
71+
'a $ dollar',
72+
'interpolation ${1 + 1} stays literal',
73+
'a $name that names no variable',
74+
'two\nlines',
75+
'carriage\rreturn',
76+
'tabs\tand\tmore',
77+
'everything: "\\ $ ${x} \n \r \t',
78+
'nul\u0000bell\u0007esc\u001b',
79+
'literal backslash-u: \\u0041',
80+
'line sep\u2028para sep\u2029',
81+
'unicode \u00e9 \u4e2d\u6587 \ud83d\ude00'].each { String text ->
82+
assert outputOf(factory.getOutputStatement(text)) == text + System.lineSeparator()
83+
}
84+
}
85+
86+
@Test
87+
void testProgramAndMethodCallSyntaxTakeCode() {
88+
// Both take code by contract, so they concatenate rather than escape.
89+
assert factory.getProgram('println("hello")', 'println("world")') ==
90+
'println("hello")\nprintln("world")\n'
91+
assert factory.getMethodCallSyntax('obj', 'foo', 'x') == 'obj.foo(x)'
92+
assert factory.getMethodCallSyntax('obj', 'foo') == 'obj.foo()'
93+
}
94+
}

0 commit comments

Comments
 (0)