diff --git a/core/src/main/java/google/registry/flows/FlowReporter.java b/core/src/main/java/google/registry/flows/FlowReporter.java index b4a6b2578..abd9585d1 100644 --- a/core/src/main/java/google/registry/flows/FlowReporter.java +++ b/core/src/main/java/google/registry/flows/FlowReporter.java @@ -15,9 +15,9 @@ package google.registry.flows; import static com.google.common.collect.ImmutableSet.toImmutableSet; +import static google.registry.flows.domain.DomainFlowUtils.toLogSafeLabel; import static java.util.Collections.EMPTY_LIST; -import com.google.common.base.Ascii; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -90,9 +90,7 @@ public class FlowReporter { */ private static Optional extractTld(String domainName) { int index = domainName.indexOf('.'); - return index == -1 - ? Optional.empty() - : Optional.of(Ascii.toLowerCase(domainName.substring(index + 1))); + return index == -1 ? Optional.empty() : toLogSafeLabel(domainName.substring(index + 1)); } /** diff --git a/core/src/main/java/google/registry/flows/domain/DomainFlowUtils.java b/core/src/main/java/google/registry/flows/domain/DomainFlowUtils.java index 33a88fa1c..2dfbcbe9a 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainFlowUtils.java +++ b/core/src/main/java/google/registry/flows/domain/DomainFlowUtils.java @@ -14,6 +14,7 @@ package google.registry.flows.domain; +import static com.google.common.base.Ascii.toLowerCase; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Strings.emptyToNull; @@ -175,6 +176,27 @@ public class DomainFlowUtils { /** Maximum number of characters in a domain label, from RFC 2181. */ private static final int MAX_LABEL_SIZE = 63; + /// Given a tld name or domain label, returns a label that can be safely logged, or + /// `Optional.empty()` if `label` is null or blank. + /// + /// If `label` contains disallowed characters, they are replaced with '-'. If the resulting + /// string has a valid length, it is returned as is; if not, the first {@link #MAX_LABEL_SIZE} + /// chars plus a suffix of `...` is returned. + /// + /// This method is mainly for use by `FlowReporter#recordToLogs()` to ensure that the logs + /// can be safely and correctly queried by SQL queries. See b/534931586 for more information. + public static Optional toLogSafeLabel(String label) { + if (label == null || label.isBlank()) { + return Optional.empty(); + } + var newLabel = ALLOWED_CHARS.negate().replaceFrom(toLowerCase(label), '-'); + if (newLabel.length() <= MAX_LABEL_SIZE) { + return Optional.of(newLabel); + } else { + return Optional.of(newLabel.substring(0, MAX_LABEL_SIZE) + "..."); + } + } + /** * Returns parsed version of {@code name} if domain name label follows our naming rules and is * under one of the given allowed TLDs. diff --git a/core/src/main/resources/google/registry/reporting/icann/sql/epp_metrics.sql b/core/src/main/resources/google/registry/reporting/icann/sql/epp_metrics.sql index 424dde567..3dacd7d18 100644 --- a/core/src/main/resources/google/registry/reporting/icann/sql/epp_metrics.sql +++ b/core/src/main/resources/google/registry/reporting/icann/sql/epp_metrics.sql @@ -21,15 +21,12 @@ -- "tld":"","tlds":["a.how"],"icannActivityReportField":"srs-dom-check"} SELECT - -- Remove quotation marks from tld fields. - REGEXP_EXTRACT(tld, '^"(.*)"$') AS tld, + tld, activityReportField AS metricName, COUNT(*) AS count FROM ( SELECT - -- TODO(b/32486667): Replace with JSON.parse() UDF when available for views - SPLIT( - REGEXP_EXTRACT(JSON_EXTRACT(json, '$.tlds'), r'^\[(.*)\]$')) AS tlds, + JSON_EXTRACT_STRING_ARRAY(json, '$.tlds') AS tlds, JSON_EXTRACT_SCALAR(json, '$.resourceType') AS resourceType, JSON_EXTRACT_SCALAR(json, @@ -43,10 +40,10 @@ FROM ( WHERE STARTS_WITH(jsonPayload.message, "FLOW-LOG-SIGNATURE-METADATA") AND _TABLE_SUFFIX BETWEEN '%FIRST_DAY_OF_MONTH%' AND '%LAST_DAY_OF_MONTH%') - ) AS regexes + ) AS json_parsed JOIN -- Unnest the JSON-parsed tlds. - UNNEST(regexes.tlds) AS tld + UNNEST(json_parsed.tlds) AS tld -- Exclude cases that can't be tabulated correctly, where activityReportField -- is null/empty, or TLD is null/empty despite being a domain flow. WHERE diff --git a/core/src/test/java/google/registry/flows/FlowReporterTest.java b/core/src/test/java/google/registry/flows/FlowReporterTest.java index 564bb5b54..547674a79 100644 --- a/core/src/test/java/google/registry/flows/FlowReporterTest.java +++ b/core/src/test/java/google/registry/flows/FlowReporterTest.java @@ -184,16 +184,22 @@ class FlowReporterTest { @Test void testRecordToLogs_metadata_invalidDomainName_stillGuessesTld() throws Exception { + var maxLenTld = "a".repeat(63); + var domainMaxLenTld = "a." + maxLenTld; + var domainTldTooLong = "a." + maxLenTld + "b"; when(flowReporter.eppInput.isDomainType()).thenReturn(true); when(flowReporter.eppInput.getSingleTargetId()).thenReturn(Optional.of("")); - when(flowReporter.eppInput.getTargetIds()).thenReturn(ImmutableList.of("")); + when(flowReporter.eppInput.getTargetIds()) + .thenReturn(ImmutableList.of("", domainMaxLenTld, domainTldTooLong)); flowReporter.recordToLogs(); Map json = parseJsonMap(findFirstLogMessageByPrefix(handler, "FLOW-LOG-SIGNATURE-METADATA: ")); assertThat(json).containsEntry("targetId", ""); - assertThat(json).containsEntry("targetIds", ImmutableList.of("")); - assertThat(json).containsEntry("tld", "com>"); - assertThat(json).containsEntry("tlds", ImmutableList.of("com>")); + assertThat(json) + .containsEntry( + "targetIds", ImmutableList.of("", domainMaxLenTld, domainTldTooLong)); + assertThat(json).containsEntry("tld", "com-"); + assertThat(json).containsEntry("tlds", ImmutableList.of("com-", maxLenTld, maxLenTld + "...")); } @Test diff --git a/core/src/test/resources/google/registry/reporting/icann/epp_metrics_test.sql b/core/src/test/resources/google/registry/reporting/icann/epp_metrics_test.sql index 1aa1bc776..0b9f29a92 100644 --- a/core/src/test/resources/google/registry/reporting/icann/epp_metrics_test.sql +++ b/core/src/test/resources/google/registry/reporting/icann/epp_metrics_test.sql @@ -21,15 +21,12 @@ -- "tld":"","tlds":["a.how"],"icannActivityReportField":"srs-dom-check"} SELECT - -- Remove quotation marks from tld fields. - REGEXP_EXTRACT(tld, '^"(.*)"$') AS tld, + tld, activityReportField AS metricName, COUNT(*) AS count FROM ( SELECT - -- TODO(b/32486667): Replace with JSON.parse() UDF when available for views - SPLIT( - REGEXP_EXTRACT(JSON_EXTRACT(json, '$.tlds'), r'^\[(.*)\]$')) AS tlds, + JSON_EXTRACT_STRING_ARRAY(json, '$.tlds') AS tlds, JSON_EXTRACT_SCALAR(json, '$.resourceType') AS resourceType, JSON_EXTRACT_SCALAR(json, @@ -43,10 +40,10 @@ FROM ( WHERE STARTS_WITH(jsonPayload.message, "FLOW-LOG-SIGNATURE-METADATA") AND _TABLE_SUFFIX BETWEEN '20170901' AND '20170930') - ) AS regexes + ) AS json_parsed JOIN -- Unnest the JSON-parsed tlds. - UNNEST(regexes.tlds) AS tld + UNNEST(json_parsed.tlds) AS tld -- Exclude cases that can't be tabulated correctly, where activityReportField -- is null/empty, or TLD is null/empty despite being a domain flow. WHERE