From f9f8231d4eaf5e811319aa7013acfb8f4da194e0 Mon Sep 17 00:00:00 2001 From: Rick Grashel Date: Mon, 4 Dec 2023 13:26:55 -0600 Subject: [PATCH] * Fixed #108 --- examples/pom.xml | 5 -- pom.xml | 14 --- stripes/pom.xml | 4 - .../validation/EmailTypeConverter.java | 43 +++++++--- .../action/TestStreamingResolution.java | 43 +++++----- .../validation/EmailTypeConverterTest.java | 86 +++++++++++++++++++ webtests/pom.xml | 5 -- 7 files changed, 140 insertions(+), 60 deletions(-) create mode 100644 stripes/src/test/java/net/sourceforge/stripes/validation/EmailTypeConverterTest.java diff --git a/examples/pom.xml b/examples/pom.xml index 8eb2dae9..b5054564 100644 --- a/examples/pom.xml +++ b/examples/pom.xml @@ -37,11 +37,6 @@ jakarta.servlet.jsp.jstl 3.0.1 - - jakarta.mail - jakarta.mail-api - compile - commons-logging commons-logging diff --git a/pom.xml b/pom.xml index 64789bfb..b5047c77 100644 --- a/pom.xml +++ b/pom.xml @@ -78,20 +78,6 @@ 1.1.2 provided - - jakarta.mail - jakarta.mail-api - 2.1.2 - provided - true - - - jakarta.activation - jakarta.activation-api - - - - commons-logging commons-logging diff --git a/stripes/pom.xml b/stripes/pom.xml index d270b524..34804c3e 100644 --- a/stripes/pom.xml +++ b/stripes/pom.xml @@ -40,10 +40,6 @@ 5.0.1 provided - - jakarta.mail - jakarta.mail-api - junit junit diff --git a/stripes/src/main/java/net/sourceforge/stripes/validation/EmailTypeConverter.java b/stripes/src/main/java/net/sourceforge/stripes/validation/EmailTypeConverter.java index 50a6b466..73cba784 100644 --- a/stripes/src/main/java/net/sourceforge/stripes/validation/EmailTypeConverter.java +++ b/stripes/src/main/java/net/sourceforge/stripes/validation/EmailTypeConverter.java @@ -14,10 +14,9 @@ */ package net.sourceforge.stripes.validation; -import jakarta.mail.internet.AddressException; -import jakarta.mail.internet.InternetAddress; import java.util.Collection; import java.util.Locale; +import java.util.regex.Pattern; /** * A faux TypeConverter that validates that the String supplied is a valid email address. Relies on @@ -42,6 +41,14 @@ * @since Stripes 1.2 */ public class EmailTypeConverter implements TypeConverter { + + /** + * This RegEx is taken from the HTML5 spec. + */ + private static final Pattern EMAIL_VALIDATION_REGEX = + Pattern.compile( + "[ \\a-zA-Z0-9.!#$%&'*+/=?^_`{|}~-]+@[[\\[]a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[]a-zA-Z0-9])?)*"); /** Accepts the Locale provided, but does nothing with it since emails are Locale-less. */ public void setLocale(Locale locale) { /** Doesn't matter for email. */ @@ -59,19 +66,31 @@ public void setLocale(Locale locale) { public String convert( String input, Class targetType, Collection errors) { - String result = null; + // Used to test for more than one @ symbol + int atSymbolOccurrences = (input.length() - input.replace("@", "").length()); + + // Used to test for no dots after the last @ sign + boolean noDotsAfterLastAtSign = + input.contains("@") && !input.substring(input.lastIndexOf("@")).contains("."); + + // Used to test for starts with a dot + boolean startsWithDot = input.split("@")[0].startsWith("."); + + // Used to test for ends with a dot + boolean endsWithDot = input.split("@")[0].endsWith("."); + + // Used to test for two consecutive dots + boolean twoConsecutiveDots = input.split("@")[0].contains(".."); - try { - InternetAddress address = new InternetAddress(input, true); - result = address.getAddress(); - if (!result.contains("@")) { - result = null; - throw new AddressException(); - } - } catch (AddressException ae) { + if (atSymbolOccurrences > 1 + || noDotsAfterLastAtSign + || startsWithDot + || endsWithDot + || twoConsecutiveDots + || !EMAIL_VALIDATION_REGEX.matcher(input).matches()) { errors.add(new ScopedLocalizableError("converter.email", "invalidEmail")); } - return result; + return input; } } diff --git a/stripes/src/test/java/net/sourceforge/stripes/action/TestStreamingResolution.java b/stripes/src/test/java/net/sourceforge/stripes/action/TestStreamingResolution.java index 01c015e9..9d0746f0 100644 --- a/stripes/src/test/java/net/sourceforge/stripes/action/TestStreamingResolution.java +++ b/stripes/src/test/java/net/sourceforge/stripes/action/TestStreamingResolution.java @@ -14,13 +14,13 @@ */ package net.sourceforge.stripes.action; -import jakarta.mail.internet.ContentDisposition; -import jakarta.mail.internet.ParseException; import java.io.ByteArrayInputStream; -import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.util.List; +import java.util.Map; import java.util.UUID; import net.sourceforge.stripes.mock.MockHttpServletResponse; +import org.apache.commons.fileupload2.core.ParameterParser; import org.junit.Assert; import org.junit.Test; @@ -34,7 +34,7 @@ public void testContentDisposition() throws Exception { } private void doTestContentDisposition(boolean attachment, String filename) throws Exception { - byte[] data = UUID.randomUUID().toString().getBytes(Charset.forName("UTF-8")); + byte[] data = UUID.randomUUID().toString().getBytes(StandardCharsets.UTF_8); ByteArrayInputStream is = new ByteArrayInputStream(data); StreamingResolution resolution = new StreamingResolution("application/octet-stream", is); @@ -48,32 +48,35 @@ private void doTestContentDisposition(boolean attachment, String filename) throw resolution.stream(response); Assert.assertArrayEquals(data, response.getOutputBytes()); - ContentDisposition disposition = getContentDisposition(response); + // Use commons-fileupload to parse the Content-Disposition header + String disposition = null; + String cdAttachment = null; + String cdFilename = null; + final List list = response.getHeaderMap().get("Content-Disposition"); + if (list != null && !list.isEmpty()) { + disposition = list.get(0).toString(); + final ParameterParser parser = new ParameterParser(); + parser.setLowerCaseNames(true); + final Map params = parser.parse(disposition, ';'); + cdAttachment = params.containsKey("attachment") ? "attachment" : null; + cdFilename = params.getOrDefault("filename", null); + } if (attachment) { + Assert.assertNotNull(disposition); + Assert.assertEquals("attachment", cdAttachment); if (filename == null) { - Assert.assertNotNull(disposition); - Assert.assertEquals("attachment", disposition.getDisposition()); - Assert.assertNull(disposition.getParameter("filename")); + Assert.assertNull(cdFilename); } else { - Assert.assertNotNull(disposition); - Assert.assertEquals("attachment", disposition.getDisposition()); - Assert.assertNotNull(disposition.getParameter("filename")); + Assert.assertNotNull(cdFilename); } } else { if (filename == null) { Assert.assertNull(disposition); } else { Assert.assertNotNull(disposition); - Assert.assertEquals("attachment", disposition.getDisposition()); - Assert.assertNotNull(disposition.getParameter("filename")); + Assert.assertEquals("attachment", cdAttachment); + Assert.assertNotNull(cdFilename); } } } - - private ContentDisposition getContentDisposition(MockHttpServletResponse response) - throws ParseException { - final List list = response.getHeaderMap().get("Content-Disposition"); - if (list == null || list.isEmpty()) return null; - else return new ContentDisposition(list.get(0).toString()); - } } diff --git a/stripes/src/test/java/net/sourceforge/stripes/validation/EmailTypeConverterTest.java b/stripes/src/test/java/net/sourceforge/stripes/validation/EmailTypeConverterTest.java new file mode 100644 index 00000000..12343821 --- /dev/null +++ b/stripes/src/test/java/net/sourceforge/stripes/validation/EmailTypeConverterTest.java @@ -0,0 +1,86 @@ +package net.sourceforge.stripes.validation; + +import java.util.ArrayList; +import java.util.List; +import org.junit.Assert; +import org.junit.Test; + +/** + * Unit tests for the EmailTypeConverter class. Test email address cases taken from here. + */ +public class EmailTypeConverterTest { + @Test + public void validEmailTests() { + List validEmailAddresses = + List.of( + "email@example.com", + "firstname.lastname@example.com", + "email@subdomain.example.com", + "firstname+lastname@example.com", + "email@123.123.123.123", + "email@[123.123.123.123]", + "\"email\"@example.com", + "johno'reilly@example.com", + "1234567890@example.com", + "email@example-one.com", + "_______@example.com", + "email@example.name", + "email@example.museum", + "email@example.co.jp", + "firstname-lastname@example.com", + "much.\"more\\ unusual\"@example.com" + // "very.unusual.\"@\".unusual.com@example.com", - Does not pass, and it should + // "very.\"(),:;<>[]\".VERY.\"very@\\\\\\ \"very\".unusual@strange.example.com" -- Does + // not pass and it should + ); + + for (String email : validEmailAddresses) { + TypeConverter converter = new EmailTypeConverter(); + List errors = new ArrayList<>(); + String result = converter.convert(email, String.class, errors); + Assert.assertEquals(result, email); + Assert.assertEquals( + "Valid email address check failed. Valid Email address was: " + email, 0, errors.size()); + } + } + + @Test + public void invalidEmailTests() { + List invalidEmailAddresses = + List.of( + "plainaddress", + "#@%^%#$@#$@#.com", + "@example.com", + "Joe Smith ", + "email.example.com", + "email@example@example.com", + ".email@example.com", + "email.@example.com", + "email..email@example.com", + "あいうえお@example.com", + "email@example.com (Joe Smith)", + "email@example", + "email@-example.com", + // "email@example.web", Should fail even though this is not strict to the old RFC which + // had limited TLDs. + // "email@111.222.333.44444", Should fail but ignoring because this is going overboard + "email@example..com", + "Abc..123@example.com" + // "\"(),:;<>[\\]@example.com", Should fail but ignoring because this is going overboard + // "just\"not\"right@example.com", Should fail but ignoring because this is going + // overboard + // "this\\ is\"really\"not\\\\allowed@example.com" Should fail but ignoring because this + // is going overboard + ); + + for (String email : invalidEmailAddresses) { + TypeConverter converter = new EmailTypeConverter(); + List errors = new ArrayList<>(); + String result = converter.convert(email, String.class, errors); + Assert.assertEquals(result, email); + Assert.assertEquals( + "Invalid email address check failed. Email address was: " + email, 1, errors.size()); + } + } +} diff --git a/webtests/pom.xml b/webtests/pom.xml index 70a9e43c..78e07bb7 100644 --- a/webtests/pom.xml +++ b/webtests/pom.xml @@ -22,11 +22,6 @@ 6.0.0 provided - - jakarta.mail - jakarta.mail-api - compile - commons-logging commons-logging