From 4c0ea766ee599360a6bb035d10eddb1ece88385e Mon Sep 17 00:00:00 2001 From: Rick Grashel Date: Mon, 4 Dec 2023 13:25:58 -0600 Subject: [PATCH 1/2] * Fixed #109 --- dist/src/assembly/upgrading.txt | 167 ++++---------------------------- 1 file changed, 20 insertions(+), 147 deletions(-) diff --git a/dist/src/assembly/upgrading.txt b/dist/src/assembly/upgrading.txt index 2b4215b21..9f4c5be10 100644 --- a/dist/src/assembly/upgrading.txt +++ b/dist/src/assembly/upgrading.txt @@ -2,9 +2,7 @@ Upgrading Stripes http://www.stripesframework.org/ This file documents the steps involved in upgrading an application from Stripes -1.4.x to Stripes 1.5. Since Stripes 1.0-1.4.3 have been backwards compatible it -should be possible to apple these steps to upgrade to Stripes 1.5 from earlier -versions also. +1.6.0 to Stripes 1.6.0-Jakarta. Contents -------- @@ -18,151 +16,26 @@ Contents of the distribution. -> Replace your existing commons-logging.jar with the new copy from the lib/ directory of the distribution - -> If you are using commons-fileupload download the latest version from the - apache website http://commons.apache.org/downloads/download_fileupload.cgi - -> Address backward compatibilities listed in section 3 as necessary - -2. Suggested Steps - -------------- - One significant new feature in Stripes 1.5 that you may wish to leverage at upgrade - time is the concept of "Extension Packages". Using this feature you can inform Stripes - of one or more extension packages, e.g.: - - Extension.Packages - com.myco.myapp.web.stripesext - - - If one or more packages are specified Stripes will search these packages for classes - that implement various interfaces and wire them up without requiring you to configure - them manually in web.xml. This includes custom: - -> ActionBeanContext classes - -> Interceptor classes - -> ConfigurableComponent classes (e.g. ActionResolvers, LocalePickers) - -> Formatter classes - -> TypeConverter classes - - To reduce your configuration it is recommended that you move any and all such - custom classes into a single package hierarchy and specify the root package. Once - done you can remove all web.xml configuration for these custom classes. - -3. Backwards Incompatibilities + -> For security reasons, Log4jLogger has been removed and replaced with SimpleJdk14Logger. + If you want to continue to use log4j, copy the source from the Stripes 1.6.0 branch + into your application source and use at your own risk. + -> If you are using cos.jar it has been removed for security reasons. Commons-fileupload2 is now + the default. Remove cos.jar and remove the CosMultipartWrapper from your web.xml if it is + referenced there. + +2. Backwards Incompatibilities --------------------------- - Stripes 1.5 is the first major release of Stripes that is not completely - backwards compatible with earlier versions of Stripes. However most of the + Stripes 1.6.0-Jakarta is not completely + backwards compatible with Stripes 1.6.0 (non-Jakarta). However, most of the incompatible change are quite small and require only minor edits to your - application (often achievable with search & replace functionality). This - section documents these incompatibilities. - - -> ActionResolver.PackageFilters has been replaced by ActionResolver.Packages - which is now a required configuration property. Changes in class scanning to - make it more efficient and robust across containers have changed the meaning - of this property and we have therefore changed the name to reflect this. If you - already specified ActionResolver.PackageFilters simply renaming it will do. - Otherwise you must specify ActionResolver.Packages to be the list of root - packages to scan for ActionBeans in your application - - -> Attribute name change in @Before and @After annotations. Previously the - lifecycle changes were specified using the 'value' attribute which - allowed the abbreviated form of '@Before(LifecycleStage.BindingAndValidation)'. - Due to the addition of the 'on' attribute to support selective execution this - is no longer desirable and the 'value' attribute has been renamed to 'stages'. - In most cases replacing '@Before(' with '@Before(stages=' (and the same for - @After) will be enough to ix this problem. - - -> The Validatable interface has been removed. Where it was used remove the - implemented clause from your ActionBean and annotate your validate() method - with the @ValidationMethod annotation - - -> The link-param tag has been removed in preference to the param tag. A search - and replace of 'link-param' to 'param' is sufficient to fix this. - - -> SpringAwareActionResolver has been removed. Please instead use the - SpringInterceptor introduced in Stripes 1.3. - - -> Stripes now performs stricter checking of various annotations. If you have - multiple methods annotated with @DefaultHandler or if you have @Validate - annotations in multiple locations (e.g. getter and setter) for a single - property Stripes will throw an exception instead of producing non-deterministic - behaviour. + application (mentioned above). This section documents these incompatibilities. - -> The _sourcePage parameter is no longer inserted into links (using the link and - url tags) by default (it is still submitted in forms). If the _sourcePage parameter - is relied on in certain places you can request Stripes insert it using the new - attribute addSourcePage="true". - - -> The _sourcePage parameter is always encrypted to avoid revealing potentially - sensitive information. The plaintext value can be easily accessed by calling - the new method ActionBeanContext.getSourcePage(). - - -> The way Interceptor classes are configured has changed. Two configuration - properties are supported: 'Interceptor.Classes' and 'CoreInterceptor.Classes'. - The first has the same name as before but the behaviour has changed. Now specifying - this property does NOT require you to specify the interceptors that Stripes - executes by default, only additional interceptors you would like to use. To - override the set of core interceptors used by Stripes you may now separately - specify the 'CoreInterceptor.Classes' property. When upgrading you should - remove referenced to the BeforeAfterMethodInterceptor from the - 'Interceptor.Classes' property. - - -> Behaviour of file uploads when using commons-fileupload has changed to match - the behaviour when using the cos implementation. When the user does not provide - a file a null FileBean will be produced instead of one containing a zero-length - file. In addition the filenames returned will never have path information - (previously path information would be included if the user was using IE). - - -> ActionClassCache has been removed. If you require a list of all ActionBeans - configured use ActionResolver.getActionBeanClasses() instead. - - -> All parameter values are now trimmed before validation, type conversion and - binding. If your app has any fields that allow whitespace as valid input or need - to retain leading and trailing whitespace during binding, you must use - @Validate(trim=false) on the field to override this behavior. - - -> The special parameter _eventName now overrides all request parameters that would - otherwise indicate the event name. E.g., given an ActionBean with events foo, bar - and blah and request parameters foo=foo, bar=bar and _eventName=blah, the event to - be executed will be blah because _eventName overrides the other two parameters. - Previous versions gave _eventName a lower precedence than other parameters. - - -> If multiple request parameters match event names, Stripes will throw an exception - reporting the conflict. In such a case, it cannot be determined which event should - execute. However, if the _eventName parameter is present, as noted in the previous - bullet, it overrides all others, even in case of such a conflict. The behavior of - previous versions of Stripes was indeterminate. - - -> When flashing an ActionBean and redirecting to that same ActionBean, as in: - - return new RedirectResolution(getClass(), "event").flash(this); - - the ActionBean receives a newly created ActionBeanContext on the ensuing request. - (In cases where the flashed bean is not the target of the redirect, the bean - retains its old ActionBeanContext.) One side-effect of this is that - ValidationErrors no longer carry over to the next request. It is recommended - that you use Messages instead of ValidationErrors when flashing and redirecting - to the same ActionBean. - - In addition to the above changes there have also been changes to several core classes. - In most cases these changes will be invisible to application developers, but where you - have implemented your own ConfigurableComponents from scratch (as opposed to extending - the default implementations) you may notice additional interface methods and changes - to signatures. Please refer to the javadoc for the 1.5 release in these cases. The - following is a non-exhaustive list of core classes with interface changes in 1.5: - -> Configuration (and implementations thereof) - -> ActionResolver (and implementations thereof) - -> BootstrapPropertyResolver - -> UrlBuilder - - -> The default PopulationStrategy is now BeanFirstPopulationStrategy instead of - DefaultPopulationStrategy. This may cause backward compatibility issues if - you were relying on the behavior of DefaultPopulationStrategy. You can revert - back to this behavior by configuring the population strategy in web.xml: - - - PopulationStrategy.Class - net.sourceforge.stripes.tag.DefaultPopulationStrategy - - - On the other hand, if you had the above configuration present in web.xml but with - BeanFirstPopulationStrategy, it is no longer required since it has become the - default. Leaving it there would be redundant, but would cause no harm. + -> Log4jLogger has been removed and the dependency on log4j has been removed. + -> cos.jar and CosMultipartWrapper have been removed. + -> commons-fileupload has been replaced with commons-fileupload2 + -> jakarta.mail dependency has been removed. +2. More Information + --------------------------- + For more information about the release, you can look at the release page on GitHub + located here: https://github.com/StripesFramework/stripes/releases/tag/1.6.0-Jakarta-beta From f9f8231d4eaf5e811319aa7013acfb8f4da194e0 Mon Sep 17 00:00:00 2001 From: Rick Grashel Date: Mon, 4 Dec 2023 13:26:55 -0600 Subject: [PATCH 2/2] * 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 8eb2dae9c..b50545640 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 64789bfb1..b5047c775 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 d270b5249..34804c3e8 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 50a6b4669..73cba7848 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 01c015e9e..9d0746f05 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 000000000..123438210 --- /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 70a9e43ce..78e07bb70 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