Improve OwnerAddressParser on strictness and test coverage

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")

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 <https://example.com>
+        if (UrlValidator.getInstance().isValid(urlOrEmail)) {
+            return new OwnerAddress(displayName, urlOrEmail);
         }
 
-        if (LOOKS_LIKE_EMAIL.matcher(address).matches()) {
-            return "mailto:" + address;
+        // e.g.: John Doe <[email protected]>
+        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("<");
     }
 }

allure-generator/src/test/java/io/qameta/allure/owner/OwnerAddressParserTest.java

@@ -32,20 +32,28 @@ void shouldReturnNullForEmptyInput() {
 
     @Test
     void shouldParseRFC2822FormattedStringWithEmail() {
-        String input = "John Doe <[email protected]>";
+        String input = "John Doe < [email protected] >";
         OwnerAddress expected = new OwnerAddress("John Doe", "mailto:[email protected]");
         assertEquals(expected.getDisplayName(), OwnerAddressParser.parseAddress(input).getDisplayName());
         assertEquals(expected.getUrl(), OwnerAddressParser.parseAddress(input).getUrl());
     }
 
     @Test
     void shouldParseRFC2822FormattedStringWithURL() {
-        String input = "John Doe <https://github.com/john.doe>";
-        OwnerAddress expected = new OwnerAddress("John Doe", "https://github.com/john.doe");
+        String input = "John Doe <https://github.com/@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 = "[email protected]";
+        OwnerAddress actual = OwnerAddressParser.parseAddress(invalidEmail);
+        assertEquals(invalidEmail, actual.getDisplayName());
+        assertNull(actual.getUrl());
+    }
+
+    @Test
+    void shouldReturnInvalidRFC822AddressUnchanged() {
+        String invalidAddress = "John Doe <john@@doe>";
+        OwnerAddress actual = OwnerAddressParser.parseAddress(invalidAddress);
+        assertEquals(invalidAddress, actual.getDisplayName());
+        assertNull(actual.getUrl());
+    }
 }