Temporarily introduces configuration property UiBinder.useSafeHtmlTemplates to
allow UiBinder's SafeHtml integration to be disabled while the last
couple of kinks are worked out.
Review at http://gwt-code-reviews.appspot.com/1402801
Review by: sbrubaker@google.com
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@9948 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/uibinder/UiBinder.gwt.xml b/user/src/com/google/gwt/uibinder/UiBinder.gwt.xml
index bdf0743..b9f2731 100644
--- a/user/src/com/google/gwt/uibinder/UiBinder.gwt.xml
+++ b/user/src/com/google/gwt/uibinder/UiBinder.gwt.xml
@@ -18,9 +18,19 @@
<source path="client"/>
<source path="resources"/>
+
<!-- Pluggable factory for creating field types for HTML elements -->
- <define-configuration-property name="uibinder.html.elementfactory" is-multi-valued="false"/>
+ <define-configuration-property name="uibinder.html.elementfactory" is-multi-valued="false"/>
<set-configuration-property name="uibinder.html.elementfactory" value="com.google.gwt.uibinder.rebind.GwtDomHtmlElementFactory"/>
+
+ <!-- By default UiBinder implementations are generated to use SafeHtmlTemplates
+ to help protect against the introduction of cross-site scripting (XSS) attacks.
+ This deprecated property can be used to disable that integration while the
+ kinks are worked out. Its use is strongly discouraged, and the property will
+ be removed in the near future. -->
+ <define-configuration-property name="UiBinder.useSafeHtmlTemplates" is-multi-valued="false"/>
+ <set-configuration-property name="UiBinder.useSafeHtmlTemplates" value="true"/>
+
<generate-with class="com.google.gwt.uibinder.rebind.UiBinderGenerator">
<when-type-assignable class="com.google.gwt.uibinder.client.UiBinder"/>
</generate-with>
diff --git a/user/src/com/google/gwt/uibinder/elementparsers/AttributeMessageInterpreter.java b/user/src/com/google/gwt/uibinder/elementparsers/AttributeMessageInterpreter.java
index abca177..9925615 100644
--- a/user/src/com/google/gwt/uibinder/elementparsers/AttributeMessageInterpreter.java
+++ b/user/src/com/google/gwt/uibinder/elementparsers/AttributeMessageInterpreter.java
@@ -46,8 +46,16 @@
throws UnableToCompleteException {
MessagesWriter messages = writer.getMessages();
for (AttributeMessage am : messages.consumeAttributeMessages(elem)) {
+ String message = am.getMessageUnescaped();
+ if (!writer.useSafeHtmlTemplates()) {
+ /*
+ * We have to do our own simple escaping to if the SafeHtml integration
+ * is off
+ */
+ message += ".replaceAll(\"&\", \"&\").replaceAll(\"'\", \"'\")";
+ }
elem.setAttribute(am.getAttribute(),
- writer.tokenForStringExpression(am.getMessageUnescaped()));
+ writer.tokenForStringExpression(message));
}
/*
diff --git a/user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java b/user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java
index 172ec11..7bbc33f 100644
--- a/user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java
+++ b/user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java
@@ -91,8 +91,12 @@
// Create an element to hold the widget.
String tag = getLegalPlaceholderTag(elem);
- return "<" + tag + " id='" + uiWriter.tokenForStringExpression(idHolder)
- + "'></" + tag + ">";
+ if (uiWriter.useSafeHtmlTemplates()) {
+ idHolder = uiWriter.tokenForStringExpression(idHolder);
+ } else {
+ idHolder = "\" + " + idHolder + " + \"";
+ }
+ return "<" + tag + " id='" + idHolder + "'></" + tag + ">";
}
return null;
}
diff --git a/user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java b/user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java
index 80f3f18..803b9bf 100644
--- a/user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java
+++ b/user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java
@@ -55,8 +55,10 @@
* message.
*/
class WidgetPlaceholderInterpreter extends HtmlPlaceholderInterpreter {
- // Could break this up into three further classes, for HasText, HasHTML
- // and Other, but that seems more trouble than it's worth.
+ /*
+ * Could break this up into three further classes, for HasText, HasHTML and
+ * Other, but that seems more trouble than it's worth.
+ */
private int serial = 0;
private final String ancestorExpression;
@@ -132,8 +134,12 @@
}
private String genOpenTag(String name, String idHolder) {
- String openTag = String.format("<span id='%s'>",
- uiWriter.tokenForStringExpression(idHolder));
+ if (uiWriter.useSafeHtmlTemplates()) {
+ idHolder = uiWriter.tokenForStringExpression(idHolder);
+ } else {
+ idHolder = "\" + " + idHolder + " + \"";
+ }
+ String openTag = String.format("<span id='%s'>", idHolder);
String openPlaceholder = nextOpenPlaceholder(name + "Begin", openTag);
return openPlaceholder;
}
@@ -180,8 +186,12 @@
}
private String handleOpaqueWidgetPlaceholder(String name, String idHolder) {
- String tag = String.format("<span id='%s'></span>",
- uiWriter.tokenForStringExpression(idHolder));
+ if (uiWriter.useSafeHtmlTemplates()) {
+ idHolder = uiWriter.tokenForStringExpression(idHolder);
+ } else {
+ idHolder = "\" + " + idHolder + " + \"";
+ }
+ String tag = String.format("<span id='%s'></span>", idHolder);
String placeholder = nextPlaceholder(name, "<span></span>", tag);
return placeholder;
}
diff --git a/user/src/com/google/gwt/uibinder/rebind/UiBinderGenerator.java b/user/src/com/google/gwt/uibinder/rebind/UiBinderGenerator.java
index 0c6e7e7..e459922 100644
--- a/user/src/com/google/gwt/uibinder/rebind/UiBinderGenerator.java
+++ b/user/src/com/google/gwt/uibinder/rebind/UiBinderGenerator.java
@@ -15,6 +15,7 @@
*/
package com.google.gwt.uibinder.rebind;
+import com.google.gwt.core.ext.BadPropertyValueException;
import com.google.gwt.core.ext.ConfigurationProperty;
import com.google.gwt.core.ext.Generator;
import com.google.gwt.core.ext.GeneratorContext;
@@ -35,6 +36,7 @@
import org.xml.sax.SAXParseException;
import java.io.PrintWriter;
+import java.util.List;
/**
* Generator for implementations of
@@ -42,10 +44,14 @@
*/
public class UiBinderGenerator extends Generator {
- private static final String TEMPLATE_SUFFIX = ".ui.xml";
-
static final String BINDER_URI = "urn:ui:com.google.gwt.uibinder";
+ private static final String TEMPLATE_SUFFIX = ".ui.xml";
+
+ private static final String ELEMENT_FACTORY_PROPERTY = "uibinder.html.elementfactory";
+
+ private static final String XSS_SAFE_CONFIG_PROPERTY = "UiBinder.useSafeHtmlTemplates";
+
/**
* Given a UiBinder interface, return the path to its ui.xml file, suitable
* for any classloader to find it as a resource.
@@ -117,30 +123,18 @@
packageName);
PrintWriter printWriter = writers.tryToMakePrintWriterFor(implName);
- Class<?> elementFactoryClass;
- HtmlElementFactory elementFactory = null;
- try {
- PropertyOracle propertyOracle = genCtx.getPropertyOracle();
- ConfigurationProperty factoryProperty = propertyOracle
- .getConfigurationProperty("uibinder.html.elementfactory");
- elementFactoryClass = Class.forName(factoryProperty.getValues().get(0));
- elementFactory = (HtmlElementFactory) elementFactoryClass.newInstance();
- } catch (Exception e) {
- throw new RuntimeException(e);
- }
-
if (printWriter != null) {
generateOnce(interfaceType, implName, printWriter, logger, oracle,
- resourceOracle, writers, designTime, elementFactory);
+ resourceOracle, genCtx.getPropertyOracle(), writers, designTime);
}
return packageName + "." + implName;
}
private void generateOnce(JClassType interfaceType, String implName,
PrintWriter binderPrintWriter, TreeLogger treeLogger, TypeOracle oracle,
- ResourceOracle resourceOracle, PrintWriterManager writerManager,
- DesignTimeUtils designTime,
- HtmlElementFactory elementFactory) throws UnableToCompleteException {
+ ResourceOracle resourceOracle, PropertyOracle propertyOracle,
+ PrintWriterManager writerManager, DesignTimeUtils designTime)
+ throws UnableToCompleteException {
MortalLogger logger = new MortalLogger(treeLogger);
String templatePath = deduceTemplateFile(logger, interfaceType);
@@ -149,7 +143,8 @@
UiBinderWriter uiBinderWriter = new UiBinderWriter(interfaceType, implName,
templatePath, oracle, logger, new FieldManager(oracle, logger),
- messages, designTime, uiBinderCtx, elementFactory);
+ messages, designTime, uiBinderCtx, getElementFactory(propertyOracle),
+ useSafeHtmlTemplates(logger, propertyOracle));
Document doc = getW3cDoc(logger, designTime, resourceOracle, templatePath);
designTime.rememberPathForElements(doc);
@@ -166,6 +161,20 @@
writerManager.commit();
}
+ private HtmlElementFactory getElementFactory(PropertyOracle propertyOracle) {
+ Class<?> elementFactoryClass;
+
+ try {
+ // TODO(cromwellian) finish this or get it out of here
+ ConfigurationProperty factoryProperty = propertyOracle
+ .getConfigurationProperty(ELEMENT_FACTORY_PROPERTY);
+ elementFactoryClass = Class.forName(factoryProperty.getValues().get(0));
+ return (HtmlElementFactory) elementFactoryClass.newInstance();
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ }
+
private Document getW3cDoc(MortalLogger logger, DesignTimeUtils designTime,
ResourceOracle resourceOracle, String templatePath)
throws UnableToCompleteException {
@@ -190,4 +199,30 @@
}
return doc;
}
+
+ private Boolean useSafeHtmlTemplates(MortalLogger logger, PropertyOracle propertyOracle) {
+ List<String> values;
+ try {
+ values = propertyOracle.getConfigurationProperty(XSS_SAFE_CONFIG_PROPERTY).getValues();
+ } catch (BadPropertyValueException e) {
+ logger.warn("No value found for configuration property %s.", XSS_SAFE_CONFIG_PROPERTY);
+ return true;
+ }
+
+ String value = values.get(0);
+ if (!value.equals(Boolean.FALSE.toString()) && !value.equals(Boolean.TRUE.toString())) {
+ logger.warn("Unparseable value \"%s\" found for configuration property %s", value,
+ XSS_SAFE_CONFIG_PROPERTY);
+ return true;
+ }
+
+ Boolean rtn = Boolean.valueOf(value);
+
+ if (!rtn) {
+ logger.warn("Configuration property %s is false! UiBinder SafeHtml integration is off, "
+ + "leaving your users more vulnerable to cross-site scripting attacks.",
+ XSS_SAFE_CONFIG_PROPERTY);
+ }
+ return rtn;
+ }
}
diff --git a/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java b/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java
index 9b6a867..95fb765 100644
--- a/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java
+++ b/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java
@@ -199,6 +199,8 @@
private final FieldManager fieldManager;
private final ImplicitClientBundle bundleClass;
+
+ private final boolean useSafeHtmlTemplates;
private int domId = 0;
@@ -232,7 +234,7 @@
String templatePath, TypeOracle oracle, MortalLogger logger,
FieldManager fieldManager, MessagesWriter messagesWriter,
DesignTimeUtils designTime, UiBinderContext uiBinderCtx,
- HtmlElementFactory elementFactory)
+ HtmlElementFactory elementFactory, boolean useSafeHtmlTemplates)
throws UnableToCompleteException {
this.baseClass = baseClass;
this.implClassName = implClassName;
@@ -244,6 +246,7 @@
this.designTime = designTime;
this.uiBinderCtx = uiBinderCtx;
this.elementFactory = elementFactory;
+ this.useSafeHtmlTemplates = useSafeHtmlTemplates;
// Check for possible misuse 'GWT.create(UiBinder.class)'
JClassType uibinderItself = oracle.findType(UiBinder.class.getCanonicalName());
@@ -414,6 +417,10 @@
*/
public String declareTemplateCall(String html)
throws IllegalArgumentException {
+ if (!useSafeHtmlTemplates) {
+ return '"' + html + '"';
+ }
+
return htmlTemplates.addSafeHtmlTemplate(html, tokenator);
}
@@ -694,6 +701,10 @@
* @param expression
*/
public String tokenForSafeHtmlExpression(String expression) {
+ if (!useSafeHtmlTemplates) {
+ return tokenForStringExpression(expression);
+ }
+
String token = tokenator.nextToken("SafeHtmlUtils.fromSafeConstant(" +
expression + ")");
htmlTemplates.noteSafeConstant("SafeHtmlUtils.fromSafeConstant(" +
@@ -716,26 +727,33 @@
}
/**
+ * @return true of SafeHtml integration is in effect
+ */
+ public boolean useSafeHtmlTemplates() {
+ return useSafeHtmlTemplates;
+ }
+
+ /**
* Post a warning message.
*/
public void warn(String message) {
logger.warn(message);
}
-
+
/**
* Post a warning message.
*/
public void warn(String message, Object... params) {
logger.warn(message, params);
}
-
+
/**
* Post a warning message.
*/
public void warn(XMLElement context, String message, Object... params) {
logger.warn(context, message, params);
}
-
+
/**
* Entry point for the code generation logic. It generates the
* implementation's superstructure, and parses the root widget (leading to all
@@ -758,7 +776,7 @@
this.rendered = tokenator.detokenate(parseDocumentElement(elem));
printWriter.print(rendered);
}
-
+
private void addElementParser(String gwtClass, String parser) {
elementParsers.put(gwtClass, parser);
}
@@ -1225,14 +1243,16 @@
w.write("package %1$s;", packageName);
w.newline();
}
- }
-
+ }
+
/**
* Write statements created by {@link HtmlTemplates#addSafeHtmlTemplate}. This
* code must be placed after all instantiation code.
*/
private void writeSafeHtmlTemplates(IndentedWriter w) {
if (!(htmlTemplates.isEmpty())) {
+ assert useSafeHtmlTemplates : "SafeHtml is off, but templates were made.";
+
w.write("interface Template extends SafeHtmlTemplates {");
w.indent();
@@ -1244,8 +1264,8 @@
w.write("Template template = GWT.create(Template.class);");
w.newline();
}
- }
-
+ }
+
/**
* Generates instances of any bundle classes that have been referenced by a
* namespace entry in the top level element. This must be called *after* all
diff --git a/user/test/com/google/gwt/uibinder/elementparsers/MockUiBinderWriter.java b/user/test/com/google/gwt/uibinder/elementparsers/MockUiBinderWriter.java
index 284880d..03dcf98 100644
--- a/user/test/com/google/gwt/uibinder/elementparsers/MockUiBinderWriter.java
+++ b/user/test/com/google/gwt/uibinder/elementparsers/MockUiBinderWriter.java
@@ -39,7 +39,7 @@
HtmlElementFactory factory) throws UnableToCompleteException {
super(baseClass, implClassName, templatePath, oracle, logger, fieldManager,
messagesWriter, DesignTimeUtilsStub.EMPTY, new UiBinderContext(),
- factory);
+ factory, true);
}
@Override