From 8b73a1722e8d5cb99a958999a3216cb8064b0bb1 Mon Sep 17 00:00:00 2001 From: Yaroslav Serhieiev Date: Thu, 9 May 2024 10:23:14 +0300 Subject: [PATCH] Improve OwnerAddressParser on strictness and test coverage --- allure-generator/build.gradle.kts | 1 + .../allure/owner/OwnerAddressParser.java | 54 +++++++++++-------- .../allure/owner/OwnerAddressParserTest.java | 38 +++++++++++-- 3 files changed, 69 insertions(+), 24 deletions(-) diff --git a/allure-generator/build.gradle.kts b/allure-generator/build.gradle.kts index 7dbd7c326..869303636 100644 --- a/allure-generator/build.gradle.kts +++ b/allure-generator/build.gradle.kts @@ -106,6 +106,7 @@ dependencies { implementation("com.fasterxml.jackson.dataformat:jackson-dataformat-yaml") implementation("com.fasterxml.jackson.module:jackson-module-jaxb-annotations") implementation("commons-io:commons-io") + implementation("commons-validator:commons-validator:1.7") implementation("io.qameta.allure:allure-model") implementation("javax.xml.bind:jaxb-api") implementation("org.allurefw:allure1-model") diff --git a/allure-generator/src/main/java/io/qameta/allure/owner/OwnerAddressParser.java b/allure-generator/src/main/java/io/qameta/allure/owner/OwnerAddressParser.java index 6e9c43683..499b0d5ad 100644 --- a/allure-generator/src/main/java/io/qameta/allure/owner/OwnerAddressParser.java +++ b/allure-generator/src/main/java/io/qameta/allure/owner/OwnerAddressParser.java @@ -15,50 +15,62 @@ */ package io.qameta.allure.owner; -import java.net.MalformedURLException; +import org.apache.commons.validator.routines.EmailValidator; +import org.apache.commons.validator.routines.UrlValidator; + import java.util.regex.Matcher; import java.util.regex.Pattern; public final class OwnerAddressParser { - private static final Pattern RFC2822_ADDRESS = Pattern.compile("^(.*) <(.*)>$"); - private static final Pattern LOOKS_LIKE_EMAIL = Pattern.compile("^[^@]+@[^@]+$"); + private static final Pattern RFC2822_ADDRESS = Pattern.compile("^([^<>]+)\\s+<\\s*(\\S*)\\s*>$"); private OwnerAddressParser() { } + @SuppressWarnings("ReturnCount") public static OwnerAddress parseAddress(final String maybeAddress) { if (maybeAddress == null || maybeAddress.isEmpty()) { return null; } + // Prevent performance degradation for plain text + if (!isLikelyAddress(maybeAddress)) { + return new OwnerAddress(maybeAddress, null); + } + + String displayName = maybeAddress; + String urlOrEmail = maybeAddress; + final Matcher matcher = RFC2822_ADDRESS.matcher(maybeAddress); if (matcher.matches()) { - final String displayName = matcher.group(1); - final String url = toHref(matcher.group(2)); - return new OwnerAddress(displayName, url); + displayName = matcher.group(1); + urlOrEmail = matcher.group(2); } - return new OwnerAddress(maybeAddress, toHref(maybeAddress)); - } + // e.g.: John Doe <> + if (urlOrEmail.isEmpty()) { + return new OwnerAddress(displayName, null); + } - private static String toHref(final String address) { - if (isValidURL(address)) { - return address; + // e.g.: John Doe + if (UrlValidator.getInstance().isValid(urlOrEmail)) { + return new OwnerAddress(displayName, urlOrEmail); } - if (LOOKS_LIKE_EMAIL.matcher(address).matches()) { - return "mailto:" + address; + // e.g.: John Doe + if (EmailValidator.getInstance().isValid(urlOrEmail)) { + return new OwnerAddress(displayName, "mailto:" + urlOrEmail); } - return null; + // Non-compliant addresses are treated as plain text + return new OwnerAddress(maybeAddress, null); } - private static boolean isValidURL(final String maybeURL) { - try { - new java.net.URL(maybeURL); - return true; - } catch (MalformedURLException e) { - return false; - } + /** + * Checks if the given string is likely to be a plain text (not an email or URL). + * Regular expressions are slow, therefore we just check for common characters. + */ + private static boolean isLikelyAddress(final String input) { + return input.contains("@") || input.contains(":") || input.contains("<"); } } diff --git a/allure-generator/src/test/java/io/qameta/allure/owner/OwnerAddressParserTest.java b/allure-generator/src/test/java/io/qameta/allure/owner/OwnerAddressParserTest.java index 1ebecedf0..9bf57440a 100644 --- a/allure-generator/src/test/java/io/qameta/allure/owner/OwnerAddressParserTest.java +++ b/allure-generator/src/test/java/io/qameta/allure/owner/OwnerAddressParserTest.java @@ -32,7 +32,7 @@ void shouldReturnNullForEmptyInput() { @Test void shouldParseRFC2822FormattedStringWithEmail() { - String input = "John Doe "; + String input = "John Doe < john.doe@example.com >"; OwnerAddress expected = new OwnerAddress("John Doe", "mailto:john.doe@example.com"); assertEquals(expected.getDisplayName(), OwnerAddressParser.parseAddress(input).getDisplayName()); assertEquals(expected.getUrl(), OwnerAddressParser.parseAddress(input).getUrl()); @@ -40,12 +40,20 @@ void shouldParseRFC2822FormattedStringWithEmail() { @Test void shouldParseRFC2822FormattedStringWithURL() { - String input = "John Doe "; - OwnerAddress expected = new OwnerAddress("John Doe", "https://github.com/john.doe"); + String input = "John Doe "; + OwnerAddress expected = new OwnerAddress("John Doe", "https://github.com/@john.doe"); assertEquals(expected.getDisplayName(), OwnerAddressParser.parseAddress(input).getDisplayName()); assertEquals(expected.getUrl(), OwnerAddressParser.parseAddress(input).getUrl()); } + @Test + void shouldReturnOnlyDisplayNameForEmptyRFC822Address() { + String emptyAddress = "John Doe <>"; + OwnerAddress actual = OwnerAddressParser.parseAddress(emptyAddress); + assertEquals("John Doe", actual.getDisplayName()); + assertNull(actual.getUrl()); + } + @Test void shouldReturnDisplayNameForPlainTextInput() { String displayName = "John Doe"; @@ -69,4 +77,28 @@ void shouldReturnDisplayNameAndUrlForValidURL() { assertEquals(expected.getDisplayName(), OwnerAddressParser.parseAddress(validUrl).getDisplayName()); assertEquals(expected.getUrl(), OwnerAddressParser.parseAddress(validUrl).getUrl()); } + + @Test + void shouldReturnOnlyDisplayNameForInvalidURL() { + String invalidUrl = "htp:/www.example.com/page"; + OwnerAddress actual = OwnerAddressParser.parseAddress(invalidUrl); + assertEquals(invalidUrl, actual.getDisplayName()); + assertNull(actual.getUrl()); + } + + @Test + void shouldReturnOnlyDisplayNameForInvalidEmail() { + String invalidEmail = "user@.example.com"; + OwnerAddress actual = OwnerAddressParser.parseAddress(invalidEmail); + assertEquals(invalidEmail, actual.getDisplayName()); + assertNull(actual.getUrl()); + } + + @Test + void shouldReturnInvalidRFC822AddressUnchanged() { + String invalidAddress = "John Doe "; + OwnerAddress actual = OwnerAddressParser.parseAddress(invalidAddress); + assertEquals(invalidAddress, actual.getDisplayName()); + assertNull(actual.getUrl()); + } }