From 7388958df754e7eecb24cc373f137b52c8b2fcb5 Mon Sep 17 00:00:00 2001 From: jianglai Date: Tue, 15 May 2018 15:51:31 -0700 Subject: [PATCH] Do not escape WHOIS output Both WhoisAction and WhoisHttpAction set the HTTP response content type to "text/plain". There is no need to defensively escape the content. In fact, by escaping the content, it creates more problems down the line. When used in a website, the response should be written into a DOM node by setting the textContent of the node, which automatically escapes the content. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=196743398 --- .../registry/whois/WhoisResponseImpl.java | 12 ++------- .../whois/DomainWhoisResponseTest.java | 2 +- .../whois/NameserverWhoisResponseTest.java | 2 +- .../whois/RegistrarWhoisResponseTest.java | 2 +- .../registry/whois/WhoisActionTest.java | 10 +++---- .../registry/whois/WhoisHttpActionTest.java | 27 +++---------------- .../whois/testdata/whois_action_domain.txt | 4 +-- .../testdata/whois_action_idn_punycode.txt | 4 +-- .../whois/testdata/whois_action_idn_utf8.txt | 4 +-- .../whois/testdata/whois_action_registrar.txt | 4 +-- .../registry/whois/testdata/whois_domain.txt | 2 +- .../testdata/whois_multiple_nameservers.txt | 4 +-- .../whois/testdata/whois_nameserver.txt | 2 +- .../whois/testdata/whois_registrar.txt | 2 +- 14 files changed, 26 insertions(+), 55 deletions(-) diff --git a/java/google/registry/whois/WhoisResponseImpl.java b/java/google/registry/whois/WhoisResponseImpl.java index f31b23ab0..d23e9df91 100644 --- a/java/google/registry/whois/WhoisResponseImpl.java +++ b/java/google/registry/whois/WhoisResponseImpl.java @@ -17,7 +17,6 @@ package google.registry.whois; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Strings.isNullOrEmpty; import static com.google.common.collect.ImmutableList.toImmutableList; -import static com.google.common.html.HtmlEscapers.htmlEscaper; import com.google.common.base.Joiner; import google.registry.model.eppcommon.Address; @@ -187,16 +186,9 @@ abstract class WhoisResponseImpl implements WhoisResponse { return emitNewline(); } - /** - * Remove potentially dangerous stuff from WHOIS output fields. - * - * - */ + /** Remove ASCII control characters like {@code \n} which could be used to forge output. */ private String cleanse(String value) { - return htmlEscaper().escape(value).replaceAll("[\\x00-\\x1f]", " "); + return value.replaceAll("[\\x00-\\x1f]", " "); } @Override diff --git a/javatests/google/registry/whois/DomainWhoisResponseTest.java b/javatests/google/registry/whois/DomainWhoisResponseTest.java index 2f476a917..ffb7d0e65 100644 --- a/javatests/google/registry/whois/DomainWhoisResponseTest.java +++ b/javatests/google/registry/whois/DomainWhoisResponseTest.java @@ -117,7 +117,7 @@ public class DomainWhoisResponseTest { new PostalInfo.Builder() .setType(PostalInfo.Type.INTERNATIONALIZED) .setName("EXAMPLE REGISTRANT") - .setOrg("EXAMPLE ORGANIZATION") + .setOrg("Tom & Jerry Corp.") .setAddress(new ContactAddress.Builder() .setStreet(ImmutableList.of("123 EXAMPLE STREET")) .setCity("ANYTOWN") diff --git a/javatests/google/registry/whois/NameserverWhoisResponseTest.java b/javatests/google/registry/whois/NameserverWhoisResponseTest.java index 3189ba3bc..593e20337 100644 --- a/javatests/google/registry/whois/NameserverWhoisResponseTest.java +++ b/javatests/google/registry/whois/NameserverWhoisResponseTest.java @@ -52,7 +52,7 @@ public class NameserverWhoisResponseTest { @Before public void setUp() { - persistNewRegistrar("example", "Example Registrar, Inc.", Registrar.Type.REAL, 8L); + persistNewRegistrar("example", "Hänsel & Gretel Registrar, Inc.", Registrar.Type.REAL, 8L); persistResource(loadRegistrar("example").asBuilder().setUrl("http://my.fake.url").build()); createTld("tld"); diff --git a/javatests/google/registry/whois/RegistrarWhoisResponseTest.java b/javatests/google/registry/whois/RegistrarWhoisResponseTest.java index e8d76fe78..96ca97574 100644 --- a/javatests/google/registry/whois/RegistrarWhoisResponseTest.java +++ b/javatests/google/registry/whois/RegistrarWhoisResponseTest.java @@ -103,7 +103,7 @@ public class RegistrarWhoisResponseTest { .build(), new RegistrarContact.Builder() .setParent(registrar) - .setName("John Geek") + .setName("Bonnie & Clyde") .setEmailAddress("johngeek@example-registrar.tld") .setPhoneNumber("+1.3105551215") .setFaxNumber("+1.3105551216") diff --git a/javatests/google/registry/whois/WhoisActionTest.java b/javatests/google/registry/whois/WhoisActionTest.java index e408b0bb9..83d142927 100644 --- a/javatests/google/registry/whois/WhoisActionTest.java +++ b/javatests/google/registry/whois/WhoisActionTest.java @@ -115,7 +115,7 @@ public class WhoisActionTest { @Test public void testRun_domainQuery_works() { Registrar registrar = - persistResource(makeRegistrar("evilregistrar", "Yes Virginia ", "bog@cat.みんな")), - persistResource(makeHostResource("ns1.cat.みんな", "1.2.3.4")), - persistResource(makeHostResource("ns2.cat.みんな", "bad:f00d:cafe::15:beef")), - persistResource(makeRegistrar("example", "Example Registrar", Registrar.State.ACTIVE)))); - newWhoisHttpAction("cat.みんな").run(); - assertThat(response.getPayload()).doesNotContain("