Unified: Don't pass an unsafe HTML string

new InlineHTML(html) is unsafe when html may be controlled by a user.
Even though this happens in a private method so we can inspect that
all its callers are safe, this is still nontrivial to reason about and
is not a best practice.

Instead, construct a SafeHtml from a String inside the setLineNumber
method, where it is clear that the only values are safe.

Change-Id: Ia69d3722dd381a310867389fbef81692831a3e8a
This commit is contained in:
Dave Borowitz
2016-04-26 12:54:32 -04:00
parent cae30b46a5
commit 7c568a5d62

View File

@@ -43,6 +43,7 @@ import com.google.gwt.user.client.ui.FlowPanel;
import com.google.gwt.user.client.ui.ImageResourceRenderer;
import com.google.gwt.user.client.ui.InlineHTML;
import com.google.gwtexpui.globalkey.client.GlobalKey;
import com.google.gwtexpui.safehtml.client.SafeHtml;
import net.codemirror.lib.CodeMirror;
import net.codemirror.lib.CodeMirror.LineHandle;
@@ -255,8 +256,9 @@ public class Unified extends DiffScreen {
cm.refresh();
}
private void setLineNumber(DisplaySide side, int cmLine, String html,
private void setLineNumber(DisplaySide side, int cmLine, Integer line,
String styleName) {
SafeHtml html = SafeHtml.asis(line != null ? line.toString() : " ");
InlineHTML gutter = new InlineHTML(html);
diffTable.add(gutter);
gutter.setStyleName(styleName);
@@ -266,12 +268,11 @@ public class Unified extends DiffScreen {
}
void setLineNumber(DisplaySide side, int cmLine, int line) {
setLineNumber(side, cmLine, String.valueOf(line),
UnifiedTable.style.unifiedLineNumber());
setLineNumber(side, cmLine, line, UnifiedTable.style.unifiedLineNumber());
}
void setLineNumberEmpty(DisplaySide side, int cmLine) {
setLineNumber(side, cmLine, " ",
setLineNumber(side, cmLine, null,
UnifiedTable.style.unifiedLineNumberEmpty());
}