첫 오픈소스 기여 (2) — 리뷰를 받고 다시 고치기

PR #1267 리뷰 → PR #1268

1편에서 올린 이메일 검증 수정에는 리뷰로 몇 가지 요청이 왔습니다. NBSP 외의 공백도 검사하고, 따옴표 안의 ASCII 공백은 여전히 허용하는지 봐 달라는 내용이었는데요. idn-email까지 고칠 때는 같은 검사를 복사하지 말고 공통 메서드로 분리하자는 제안도 함께 있었습니다.

idn-email은 국제화된 이메일 주소를 나타내는 JSON Schema 형식으로, münchen@example.com의 ü처럼 ASCII가 아닌 문자도 다룹니다. 첫 PR에서 고친 것은 email뿐이라 이쪽에는 주소 앞의 NBSP가 통과하는 문제가 그대로 남아 있었죠. 이번에는 그 문제를 고치면서 리뷰 내용도 함께 반영해 PR #1268을 열었습니다.

같은 검사, 다른 입력

공백 검사 코드는 EmailFormat에서 Strings.containsNonAsciiWhitespace()로 옮겨 두 형식이 함께 쓰도록 했습니다. 다만 idn-email에서는 공백을 막다가 ü 같은 글자까지 거부하면 안 되는데요. münchen@example.com처럼 U+00FC(ü)가 들어간 주소는 그대로 통과하는지도 테스트에 넣었습니다. 추가로 어떤 공백을 검사할지 고를 때는 AI로 관련 자료를 정리하고, Character.isWhitespace()와 Character.isSpaceChar()가 각각 어떤 문자를 공백으로 보는지 비교해 봤습니다.

Java 21에서 두 메서드 중 하나라도 공백으로 판단하는 문자는 28개입니다. 아래에는 이번 테스트와 관련된 사례만 추렸습니다.

문자	isWhitespace()	isSpaceChar()	테스트에서 확인할 내용
U+0020 · 일반 공백	true	true	ASCII 공백은 기존 검증기에 맡김
U+00A0 · NBSP	false	true	처음 보고된 버그. 기존 테스트에 포함
..	..	..	..
U+2003 · EM SPACE	true	true	추가한 유니코드 공백 사례
U+3000 · 전각 공백	true	true	추가한 유니코드 공백 사례

NBSP는 글자 사이를 띄우면서도 그 자리에서 줄이 바뀌지 않게 하는 문자입니다. 화면에서는 공백으로 보이지만, Java의 isWhitespace()는 NBSP를 공백으로 판단하지 않아 false를 반환합니다. Character.isWhitespace() 문서에는 U+2007과 U+202F도 같은 경우로 나와 있습니다. 그래서 이 문자들까지 걸러내려면 isSpaceChar()도 함께 써야 했습니다.

따옴표 안의 공백을 테스트하려니 입력부터 제대로 만들 수 없었습니다. 기존 코드는 이메일 앞뒤에 따옴표를 붙여 JSON 문자열을 만들었는데, "joe bloggs"@example.com을 넣으면 주소 안의 따옴표와 겹쳐 JSON이 깨지는 거죠. 직접 문자열을 조합하는 대신 JsonMapperFactory.getInstance().writeValueAsString(email)로 직렬화하도록 바꿨습니다.

한쪽 테스트에만 넣었던 공백

EmailFormatTest에는 U+2003과 U+3000을 추가하면서, 새로 만든 IdnEmailFormatTest에는 NBSP만 넣었습니다. 같은 검사 메서드를 쓰는데 테스트 입력은 서로 달랐던 셈이죠. 양쪽에서 같은 입력을 확인해야 한다는 의견을 받아 IdnEmailFormatTest에도 두 사례를 추가했습니다. 아래 테스트는 @ValueSource의 각 값을 이메일 앞에 붙여 검증 오류가 나는지 확인합니다.

@ParameterizedTest
@ValueSource(ints = { 0x2003, 0x3000 })
void idnEmailWithLeadingUnicodeWhitespaceShouldFail(int codePoint) {
    String whitespace = new String(Character.toChars(codePoint));
    List<Error> messages = validateIdnEmail(whitespace + "name@email.com");
    assertFalse(messages.isEmpty(), "idn-email with a leading non-ASCII whitespace character should be invalid");
}

테스트 헬퍼인 validateEmail()과 validateIdnEmail()도 합칠 수 있지 않겠냐는 제안이 있었습니다. 두 형식이 쓰는 공백 검사 로직은 이미 공통 메서드로 옮겼지만, 테스트 헬퍼까지 합치는 건 같은 검사가 필요한 형식이 더 생기면 하기로 했습니다. 스트림을 반복문으로 바꾸는 제안도 성능 측정에서 병목이 드러나면 검토하기로 하고, 이번에 반영한 내용과 남겨 둔 이유를 답글에 적었습니다.

리뷰어가 공백 검사 코드를 빼고 실행했을 때는 NBSP·U+2003·U+3000 테스트가 모두 실패했습니다.

다만 따옴표 안의 공백은 더 살펴봐야 했습니다. 이번 검사는 문자열 전체에서 유니코드 공백을 찾기 때문에, 이메일 맨 앞에 붙은 NBSP뿐 아니라 "joe<NBSP>bloggs"@example.com처럼 따옴표 안에 들어간 NBSP도 거부합니다. 여기서 <NBSP>는 눈에 보이지 않는 문자의 위치를 표시한 것입니다. 리뷰어는 이런 주소가 국제화된 이메일 규칙에서는 허용될 수 있다고 지적했습니다. 이 경우까지 구분하려면 따옴표 안인지도 판단해야 해서 이번 수정에는 포함하지 않았습니다. ASCII 공백을 허용하는 테스트도 EmailFormatTest에만 남아 있었습니다.