New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Qute - escape expressions in HTML by default #6194
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
package io.quarkus.qute.deployment; | ||
|
||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||
|
||
import javax.inject.Inject; | ||
|
||
import org.jboss.shrinkwrap.api.ShrinkWrap; | ||
import org.jboss.shrinkwrap.api.asset.StringAsset; | ||
import org.jboss.shrinkwrap.api.spec.JavaArchive; | ||
import org.junit.jupiter.api.Test; | ||
import org.junit.jupiter.api.extension.RegisterExtension; | ||
|
||
import io.quarkus.qute.RawString; | ||
import io.quarkus.qute.Template; | ||
import io.quarkus.qute.TemplateData; | ||
import io.quarkus.test.QuarkusUnitTest; | ||
|
||
public class EscapingTest { | ||
|
||
@RegisterExtension | ||
static final QuarkusUnitTest config = new QuarkusUnitTest() | ||
.setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class) | ||
.addClass(Item.class) | ||
.addAsResource(new StringAsset("{text} {other} {text.raw} {text.safe} {item.foo}"), | ||
"templates/foo.html") | ||
.addAsResource(new StringAsset("{item} {item.raw}"), | ||
"templates/item.html") | ||
.addAsResource(new StringAsset("{text} {other} {text.raw} {text.safe} {item.foo}"), | ||
"templates/bar.txt")); | ||
|
||
@Inject | ||
Template foo; | ||
|
||
@Inject | ||
Template bar; | ||
|
||
@Inject | ||
Template item; | ||
|
||
@Test | ||
public void testEscaper() { | ||
assertEquals("<div> &"' <div> <div> <span>", | ||
foo.data("text", "<div>").data("other", "&\"'").data("item", new Item()).render()); | ||
// No escaping for txt templates | ||
assertEquals("<div> &\"' <div> <div> <span>", | ||
bar.data("text", "<div>").data("other", "&\"'").data("item", new Item()).render()); | ||
// Item.toString() is escaped too | ||
assertEquals("<h1>Item</h1> <h1>Item</h1>", | ||
item.data("item", new Item()).render()); | ||
} | ||
|
||
@TemplateData | ||
public static class Item { | ||
|
||
public RawString getFoo() { | ||
return new RawString("<span>"); | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return "<h1>Item</h1>"; | ||
} | ||
|
||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
package io.quarkus.qute.api; | ||
|
||
import io.quarkus.qute.Template; | ||
import io.quarkus.qute.Variant; | ||
|
||
/** | ||
* | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,9 @@ | ||
package io.quarkus.qute.runtime; | ||
|
||
import java.io.InputStream; | ||
import java.io.IOException; | ||
import java.io.InputStreamReader; | ||
import java.io.Reader; | ||
import java.net.URL; | ||
import java.nio.charset.Charset; | ||
import java.util.List; | ||
import java.util.Optional; | ||
|
@@ -19,12 +20,19 @@ | |
import io.quarkus.arc.InstanceHandle; | ||
import io.quarkus.qute.Engine; | ||
import io.quarkus.qute.EngineBuilder; | ||
import io.quarkus.qute.Escaper; | ||
import io.quarkus.qute.Expression; | ||
import io.quarkus.qute.NamespaceResolver; | ||
import io.quarkus.qute.RawString; | ||
import io.quarkus.qute.ReflectionValueResolver; | ||
import io.quarkus.qute.ResultMapper; | ||
import io.quarkus.qute.Results.Result; | ||
import io.quarkus.qute.TemplateLocator.TemplateLocation; | ||
import io.quarkus.qute.TemplateNode.Origin; | ||
import io.quarkus.qute.UserTagSectionHelper; | ||
import io.quarkus.qute.ValueResolver; | ||
import io.quarkus.qute.ValueResolvers; | ||
import io.quarkus.qute.Variant; | ||
|
||
@Singleton | ||
public class EngineProducer { | ||
|
@@ -59,8 +67,31 @@ void init(QuteConfig config, List<String> resolverClasses, List<String> template | |
|
||
// We don't register the map resolver because of param declaration validation | ||
// See DefaultTemplateExtensions | ||
builder.addValueResolvers(ValueResolvers.thisResolver(), ValueResolvers.orResolver(), ValueResolvers.trueResolver(), | ||
ValueResolvers.collectionResolver(), ValueResolvers.mapperResolver(), ValueResolvers.mapEntryResolver()); | ||
builder.addValueResolver(ValueResolvers.thisResolver()); | ||
builder.addValueResolver(ValueResolvers.orResolver()); | ||
builder.addValueResolver(ValueResolvers.trueResolver()); | ||
builder.addValueResolver(ValueResolvers.collectionResolver()); | ||
builder.addValueResolver(ValueResolvers.mapperResolver()); | ||
builder.addValueResolver(ValueResolvers.mapEntryResolver()); | ||
// foo.string.raw returns a RawString which is never escaped | ||
builder.addValueResolver(ValueResolvers.rawResolver()); | ||
|
||
// Escape some characters for HTML templates | ||
Escaper htmlEscaper = Escaper.builder().add('"', """).add('\'', "'") | ||
.add('&', "&").add('<', "<").add('>', ">").build(); | ||
builder.addResultMapper(new ResultMapper() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we sure it's the last mapper? I don't think users should be able to register mappers after this one, no? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mappers with higher priority are tried first. So if a user wants to map some object then the priority should be > |
||
|
||
@Override | ||
public boolean appliesTo(Origin origin, Object result) { | ||
return !(result instanceof RawString) | ||
&& origin.getVariant().filter(EngineProducer::requiresDefaultEscaping).isPresent(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm guessing in the future we'll need to turn this into an SPI for custom escaping of other variant content types. But it can wait. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 for proper SPI... in the furure ;-). |
||
} | ||
|
||
@Override | ||
public String map(Object result, Expression expression) { | ||
return htmlEscaper.escape(result.toString()); | ||
} | ||
}); | ||
|
||
// Fallback reflection resolver | ||
builder.addValueResolver(new ReflectionValueResolver()); | ||
|
@@ -132,37 +163,79 @@ private ValueResolver createResolver(String resolverClassName) { | |
* @param path | ||
* @return the optional reader | ||
*/ | ||
private Optional<Reader> locate(String path) { | ||
InputStream in = null; | ||
private Optional<TemplateLocation> locate(String path) { | ||
URL resource = null; | ||
// First try to locate a tag template | ||
if (tags.stream().anyMatch(tag -> tag.startsWith(path))) { | ||
LOGGER.debugf("Locate tag for %s", path); | ||
in = locatePath(tagPath + path); | ||
resource = locatePath(tagPath + path); | ||
// Try path with suffixes | ||
for (String suffix : suffixes) { | ||
in = locatePath(tagPath + path + "." + suffix); | ||
if (in != null) { | ||
resource = locatePath(tagPath + path + "." + suffix); | ||
if (resource != null) { | ||
break; | ||
} | ||
} | ||
} | ||
if (in == null) { | ||
if (resource == null) { | ||
String templatePath = basePath + path; | ||
LOGGER.debugf("Locate template for %s", templatePath); | ||
in = locatePath(templatePath); | ||
resource = locatePath(templatePath); | ||
} | ||
if (in != null) { | ||
return Optional.of(new InputStreamReader(in, Charset.forName("utf-8"))); | ||
if (resource != null) { | ||
return Optional.of(new ResourceTemplateLocation(resource, guessVariant(path))); | ||
} | ||
return Optional.empty(); | ||
} | ||
|
||
private InputStream locatePath(String path) { | ||
private URL locatePath(String path) { | ||
ClassLoader cl = Thread.currentThread().getContextClassLoader(); | ||
if (cl == null) { | ||
cl = EngineProducer.class.getClassLoader(); | ||
} | ||
return cl.getResourceAsStream(path); | ||
return cl.getResource(path); | ||
} | ||
|
||
static Variant guessVariant(String path) { | ||
// TODO we need a proper way to detect the variant | ||
int suffixIdx = path.lastIndexOf('.'); | ||
if (suffixIdx != -1) { | ||
String suffix = path.substring(suffixIdx); | ||
return new Variant(null, VariantTemplateProducer.parseMediaType(suffix), null); | ||
} | ||
return null; | ||
} | ||
|
||
static boolean requiresDefaultEscaping(Variant variant) { | ||
return variant.mediaType != null | ||
? (Variant.TEXT_HTML.equals(variant.mediaType) || Variant.TEXT_XML.equals(variant.mediaType)) | ||
: false; | ||
} | ||
|
||
static class ResourceTemplateLocation implements TemplateLocation { | ||
|
||
private final URL resource; | ||
private final Optional<Variant> variant; | ||
|
||
public ResourceTemplateLocation(URL resource, Variant variant) { | ||
this.resource = resource; | ||
this.variant = Optional.ofNullable(variant); | ||
} | ||
|
||
@Override | ||
public Reader read() { | ||
try { | ||
return new InputStreamReader(resource.openStream(), Charset.forName("utf-8")); | ||
} catch (IOException e) { | ||
return null; | ||
} | ||
} | ||
|
||
@Override | ||
public Optional<Variant> getVariant() { | ||
return variant; | ||
} | ||
|
||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ | |
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Map.Entry; | ||
import java.util.Optional; | ||
import java.util.Set; | ||
import java.util.concurrent.CompletionStage; | ||
import java.util.function.Consumer; | ||
|
@@ -27,8 +28,8 @@ | |
import io.quarkus.qute.Template; | ||
import io.quarkus.qute.TemplateInstance; | ||
import io.quarkus.qute.TemplateInstanceBase; | ||
import io.quarkus.qute.Variant; | ||
import io.quarkus.qute.api.ResourcePath; | ||
import io.quarkus.qute.api.Variant; | ||
import io.quarkus.qute.api.VariantTemplate; | ||
|
||
@Singleton | ||
|
@@ -114,6 +115,11 @@ public String getGeneratedId() { | |
throw new UnsupportedOperationException(); | ||
} | ||
|
||
@Override | ||
public Optional<Variant> getVariant() { | ||
throw new UnsupportedOperationException(); | ||
} | ||
|
||
} | ||
|
||
class VariantTemplateInstanceImpl extends TemplateInstanceBase { | ||
|
@@ -166,15 +172,15 @@ public TemplateVariants(Map<Variant, String> variants, String defaultTemplate) { | |
} | ||
|
||
static String parseMediaType(String suffix) { | ||
// TODO support more media types... | ||
if (suffix.equalsIgnoreCase(".html")) { | ||
return "text/html"; | ||
// TODO we need a proper way to parse the media type | ||
if (suffix.equalsIgnoreCase(".html") || suffix.equalsIgnoreCase(".htm")) { | ||
return Variant.TEXT_HTML; | ||
} else if (suffix.equalsIgnoreCase(".xml")) { | ||
return "text/xml"; | ||
return Variant.TEXT_XML; | ||
} else if (suffix.equalsIgnoreCase(".txt")) { | ||
return "text/plain"; | ||
return Variant.TEXT_PLAIN; | ||
} else if (suffix.equalsIgnoreCase(".json")) { | ||
return "application/json"; | ||
return Variant.APPLICATION_JSON; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We support JSON but I didn't see any escaper for JSON. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, actually we don't support JSON ATM ;-). TBD |
||
} | ||
LOGGER.warn("Unknown media type for suffix: " + suffix); | ||
return "application/octet-stream"; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://en.wikipedia.org/wiki/List_of_XML_and_HTML_character_entity_references#Predefined_entities_in_XML says
'
is available for both xml and html.