Skip to content

Commit 57a4c72

Browse files
authored
PDJB-904: align notify and app email validation (#1351)
1 parent 518032c commit 57a4c72

4 files changed

Lines changed: 163 additions & 15 deletions

File tree

src/main/kotlin/uk/gov/communities/prsdb/webapp/validation/EmailConstraintValidator.kt

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,51 @@
11
package uk.gov.communities.prsdb.webapp.validation
22

3-
import org.hibernate.validator.internal.constraintvalidators.bv.EmailValidator
3+
import java.net.IDN
44

55
open class EmailConstraintValidator : PropertyConstraintValidator {
6-
override fun isValid(value: Any?): Boolean = EmailValidator().isValid(value as? CharSequence, null)
6+
companion object {
7+
private const val VALID_LOCAL_CHARS = """a-zA-Z0-9.!#$%&'*+/=?^_`{|}~\-"""
8+
private val EMAIL_REGEX = Regex("^[$VALID_LOCAL_CHARS]+@([^.@][^@\\s]+)$")
9+
private val HOSTNAME_PART_REGEX = Regex("^(xn|[a-z0-9]+)(-?-[a-z0-9]+)*$", RegexOption.IGNORE_CASE)
10+
private val TLD_PART_REGEX = Regex("^([a-z]{2,63}|xn--([a-z0-9]+-)*[a-z0-9]+)$", RegexOption.IGNORE_CASE)
11+
private const val MAX_EMAIL_LENGTH = 320
12+
private const val MAX_HOSTNAME_LENGTH = 253
13+
private const val MAX_HOSTNAME_PART_LENGTH = 63
14+
}
15+
16+
// More or less a copy of
17+
// https://github.com/alphagov/notifications-utils/blob/995faf0d925f7e95ecac6ecec383b1d162b2eceb/notifications_utils/recipient_validation/email_address.py
18+
// From Notify
19+
override fun isValid(value: Any?): Boolean {
20+
val email = (value as? CharSequence)?.toString() ?: return false
21+
if (email.isBlank()) return false
22+
23+
val match = EMAIL_REGEX.matchEntire(email) ?: return false
24+
25+
if (email.length > MAX_EMAIL_LENGTH) return false
26+
27+
if (".." in email) return false
28+
29+
val hostname =
30+
try {
31+
IDN.toASCII(match.groupValues[1])
32+
} catch (_: IllegalArgumentException) {
33+
return false
34+
}
35+
36+
if (hostname.length > MAX_HOSTNAME_LENGTH) return false
37+
38+
val parts = hostname.split(".")
39+
if (parts.size < 2) return false
40+
41+
for (part in parts) {
42+
if (part.isEmpty() || part.length > MAX_HOSTNAME_PART_LENGTH || !HOSTNAME_PART_REGEX.matches(part)) {
43+
return false
44+
}
45+
}
46+
47+
return TLD_PART_REGEX.matches(parts.last())
48+
}
749
}
850

951
class OptionalEmailConstraintValidator : EmailConstraintValidator() {

src/main/kotlin/uk/gov/communities/prsdb/webapp/validation/IsValidPrioritisedValidator.kt

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,13 @@ import kotlin.reflect.full.valueParameters
1515
* Searches for all properties on the class of the instance which have @ValidatedBy annotations (or annotations
1616
* that are themselves annotated with @ValidatedBy, i.e. composed annotations), then checks each of the
1717
* constraints in order; if one fails, the message key for that constraint is added to the field and the
18-
* validator moves on to the next @ValidatedBy annotation.
18+
* validator moves on to the next property.
1919
*
2020
* Composed annotations are supported: an annotation A can itself be annotated with @ValidatedBy (or with
2121
* another composed annotation), and placing A on a property will include those constraints. All @ValidatedBy
22-
* annotations are discovered via depth-first traversal following annotation declaration order, so the order
23-
* in which constraints are checked matches the order in which annotations (and their meta-annotations) are
24-
* declared.
22+
* annotations are discovered via depth-first traversal following annotation declaration order. Only one
23+
* violation is produced per property: the first failing constraint stops evaluation of remaining constraints
24+
* for that property.
2525
*/
2626
class IsValidPrioritisedValidator : ConstraintValidator<IsValidPrioritised, Any> {
2727
override fun isValid(
@@ -32,6 +32,7 @@ class IsValidPrioritisedValidator : ConstraintValidator<IsValidPrioritised, Any>
3232

3333
for (property in instance!!::class.memberProperties) {
3434
for (validatedBy in property.getValidatedByAnnotations()) {
35+
var propertyHasViolation = false
3536
for (constraint in validatedBy.constraints) {
3637
val validationPassed =
3738
when {
@@ -59,9 +60,11 @@ class IsValidPrioritisedValidator : ConstraintValidator<IsValidPrioritised, Any>
5960
}
6061
if (!validationPassed) {
6162
isValid = false
63+
propertyHasViolation = true
6264
break
6365
}
6466
}
67+
if (propertyHasViolation) break
6568
}
6669
}
6770

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
package uk.gov.communities.prsdb.webapp.validation
2+
3+
import org.junit.jupiter.api.Named
4+
import org.junit.jupiter.api.Nested
5+
import org.junit.jupiter.api.Test
6+
import org.junit.jupiter.params.ParameterizedTest
7+
import org.junit.jupiter.params.provider.MethodSource
8+
import kotlin.test.assertFalse
9+
import kotlin.test.assertTrue
10+
11+
class EmailConstraintValidatorTests {
12+
companion object {
13+
@JvmStatic
14+
fun provideValidEmails() =
15+
arrayOf(
16+
Named.of("simple address", "test@example.com"),
17+
Named.of("with dots in local part", "first.last@example.com"),
18+
Named.of("with plus in local part", "user+tag@example.com"),
19+
Named.of("with hyphen in domain", "user@my-domain.com"),
20+
Named.of("with subdomain", "user@mail.example.com"),
21+
Named.of("with apostrophe in local part", "firstname-o'surname@domain.com"),
22+
Named.of("with special chars in local part", "user!#\$%&'*+/=?^_`{|}~@example.com"),
23+
Named.of("single char local part", "a@example.com"),
24+
Named.of("numeric local part", "123@example.com"),
25+
Named.of("long TLD", "user@example.museum"),
26+
Named.of("two letter TLD", "user@example.uk"),
27+
)
28+
29+
@JvmStatic
30+
fun provideInvalidEmails() =
31+
arrayOf(
32+
Named.of("null value", null),
33+
Named.of("miscellaneous whitespace", " \u180e \u200b"),
34+
Named.of("empty string", ""),
35+
Named.of("no @ sign", "userexample.com"),
36+
Named.of("no domain", "user@"),
37+
Named.of("no local part", "@example.com"),
38+
Named.of("consecutive dots in local part", "user..name@example.com"),
39+
Named.of("consecutive dots in domain", "user@example..com"),
40+
Named.of("domain starts with dot", "user@.example.com"),
41+
Named.of("no TLD", "user@localhost"),
42+
Named.of("space in address", "user @example.com"),
43+
Named.of("obscure whitespace in address", "user\u180e@example.com"),
44+
Named.of("double quote in local part", "user\"name@example.com"),
45+
Named.of("semicolon in local part", "user;name@example.com"),
46+
Named.of("single letter TLD", "user@example.a"),
47+
Named.of("numeric TLD", "user@example.123"),
48+
Named.of("multiple @ signs", "user@@example.com"),
49+
Named.of("domain part over 63 chars", "user@${"a".repeat(64)}.com"),
50+
)
51+
}
52+
53+
@ParameterizedTest(name = "{0}")
54+
@MethodSource("provideValidEmails")
55+
fun `isValid returns true for valid email with`(input: String) {
56+
assertTrue(EmailConstraintValidator().isValid(input))
57+
}
58+
59+
@ParameterizedTest(name = "{0}")
60+
@MethodSource("provideInvalidEmails")
61+
fun `isValid returns false for invalid email with`(input: String?) {
62+
assertFalse(EmailConstraintValidator().isValid(input))
63+
}
64+
65+
@Test
66+
fun `isValid returns false for email longer than 320 characters`() {
67+
val longLocal = "a".repeat(310)
68+
val email = "$longLocal@example.com"
69+
assertTrue(email.length > 320)
70+
assertFalse(EmailConstraintValidator().isValid(email))
71+
}
72+
73+
@Test
74+
fun `isValid returns false for hostname longer than 253 characters`() {
75+
val longHostname = (1..42).joinToString(".") { "abcde" }
76+
val email = "user@$longHostname.com"
77+
assertFalse(EmailConstraintValidator().isValid(email))
78+
}
79+
80+
@Nested
81+
inner class OptionalEmailConstraintValidatorTests {
82+
@Test
83+
fun `isValid returns true for null`() {
84+
assertTrue(OptionalEmailConstraintValidator().isValid(null))
85+
}
86+
87+
@Test
88+
fun `isValid returns true for blank string`() {
89+
assertTrue(OptionalEmailConstraintValidator().isValid(""))
90+
assertTrue(OptionalEmailConstraintValidator().isValid(" "))
91+
}
92+
93+
@Test
94+
fun `isValid returns true for valid email`() {
95+
assertTrue(OptionalEmailConstraintValidator().isValid("user@example.com"))
96+
}
97+
98+
@Test
99+
fun `isValid returns false for invalid email`() {
100+
assertFalse(OptionalEmailConstraintValidator().isValid("not-an-email"))
101+
}
102+
}
103+
}

src/test/kotlin/uk/gov/communities/prsdb/webapp/validation/IsValidPrioritisedValidatorTest.kt

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,14 @@ import org.junit.jupiter.api.assertThrows
1616
@Retention(AnnotationRetention.RUNTIME)
1717
@ValidatedBy(
1818
constraints = [
19-
ConstraintDescriptor(validatorType = NotBlankConstraintValidator::class, messageKey = "notblank"),
19+
ConstraintDescriptor(validatorType = LengthConstraintValidator::class, validatorArgs = ["0", "7"], messageKey = "toolong"),
2020
],
2121
)
22-
annotation class SharedNotBlankValidation
22+
annotation class SharedMaxLengthValidation
2323

2424
@Target(AnnotationTarget.PROPERTY, AnnotationTarget.ANNOTATION_CLASS)
2525
@Retention(AnnotationRetention.RUNTIME)
26-
@SharedNotBlankValidation
26+
@SharedMaxLengthValidation
2727
@ValidatedBy(
2828
constraints = [
2929
ConstraintDescriptor(validatorType = EmailConstraintValidator::class, messageKey = "notemail"),
@@ -128,28 +128,28 @@ class IsValidPrioritisedValidatorTest {
128128
inner class ComposedAnnotationConstraintPropertyTests {
129129
@Test
130130
fun `no violations for object with satisfied constraints from composed annotations`() {
131-
val instance = ComposedAnnotationConstraintProperty("test@example.com")
131+
val instance = ComposedAnnotationConstraintProperty("a@b.com")
132132

133133
val violations = validator.validate(instance)
134134

135135
assertTrue(violations.isEmpty())
136136
}
137137

138138
@Test
139-
fun `first violation comes from constraints earlier in declaration order, even via composed annotations`() {
140-
val instance = ComposedAnnotationConstraintProperty("")
139+
fun `violation comes from constraints earlier in declaration order, even via composed annotations`() {
140+
val instance = ComposedAnnotationConstraintProperty("toolongAndNotAnEmail")
141141

142142
val violations = validator.validate(instance)
143143

144144
assertEquals(1, violations.size)
145145
val violation = violations.first()
146-
assertEquals("notblank", violation.messageTemplate)
146+
assertEquals("toolong", violation.messageTemplate)
147147
assertEquals("email", violation.propertyPath.toString())
148148
}
149149

150150
@Test
151-
fun `later violation comes from constraints later in declaration order when earlier ones pass`() {
152-
val instance = ComposedAnnotationConstraintProperty("not an email")
151+
fun `violation comes from constraints later in declaration order when earlier ones pass`() {
152+
val instance = ComposedAnnotationConstraintProperty("short")
153153

154154
val violations = validator.validate(instance)
155155

0 commit comments

Comments
 (0)