ProductLink 파싱 검증을 도메인 예외로 분리#118
Merged
Merged
Conversation
`GlobalExceptionHandler.handleIllegalArgument` 가 `e.message` 를 응답 detail · 로그 양쪽에 그대로 박는 구조라, `ProductLink.parse` 가 원본 URL 을 메시지에 담으면 쿼리스트링/fragment 에 섞일 수 있는 토큰·세션이 외부로 새는 경로가 됐다. `IllegalArgumentException` 으로 던지면 일반 핸들러 정책에 묶여 detail 노출이 도메인에서 통제되지 않는다. 코드베이스 컨벤션 (`WishException`, `ProductExtractionException`) 처럼 도메인 전용 예외 + companion object 정적 팩토리 메서드 패턴으로 분리한다. - `ProductLinkException` 추가 (BaseException + HttpMappable, HTTP 400 + INVALID_INPUT). 정적 팩토리: `blank()`, `invalidFormat(cause)`, `unsupportedScheme()`. `invalidFormat` 은 원본을 message 에 박지 않고 cause 로만 연결해 stack trace 에 남긴다. - `ProductLink.parse` 의 require · throw 를 모두 새 예외로 전환. - 단위 테스트 (`ProductLinkTest`): 원본 raw 가 message 에 포함되지 않음 명시 단언. - 통합 테스트 (`WishlistControllerIntegrationTest`): 응답 contract (status·code·detail) 까지 회귀 검증.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughURL 파싱 검증을 ProductLinkException으로 통합하고 고정된 예외 메시지를 사용하여 민감한 정보 노출을 방지합니다. 단위 및 통합 테스트로 토큰 유출 방지를 검증합니다. ChangesProductLink 파싱 보안 개선
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
리뷰 포인트👍 잘 구현된 부분짧고 굵게: 예외 설계로 실수 노출 가능성을 구조적으로 막은 점, 테스트로 회귀 방지를 명시한 점 모두 굿. 고려할 사항 (구체적 조치 제안)
짧고 위트있게: 안전한 예외처리, 칭찬드립니다 — 토큰을 숨기는 데 성공했어요. 🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
No description provided. |
|
:) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/test/kotlin/com/depromeet/team3/product/domain/ProductLinkTest.kt (1)
41-48: ⚡ Quick win테스트 단언 일관성 개선
Line 47의
startsWith검증은 Line 59의 새로운 테스트와 일관성이 없습니다.ProductLinkException.invalidFormat의 메시지는 고정되어 있으므로 ("유효한 URL 형식이 아닙니다.") 정확한 일치를 검증하는 것이 더 명확하고 회귀 방지에 유리합니다.♻️ 제안하는 수정
- assertTrue(ex.message?.startsWith("유효한 URL 형식이 아닙니다") == true) + assertEquals("유효한 URL 형식이 아닙니다.", ex.message)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/kotlin/com/depromeet/team3/product/domain/ProductLinkTest.kt` around lines 41 - 48, The assertion in the test `URI 파싱 자체가 실패하는 raw 는 형식 오류로 거부한다` should check for exact message equality rather than startsWith for `ProductLinkException.invalidFormat`; update the assertion that inspects ex.message (raised from ProductLink.parse) to assert equality with the fixed message "유효한 URL 형식이 아닙니다." so it matches the canonical ProductLinkException.invalidFormat text and aligns with the other test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/test/kotlin/com/depromeet/team3/product/domain/ProductLinkTest.kt`:
- Around line 41-48: The assertion in the test `URI 파싱 자체가 실패하는 raw 는 형식 오류로
거부한다` should check for exact message equality rather than startsWith for
`ProductLinkException.invalidFormat`; update the assertion that inspects
ex.message (raised from ProductLink.parse) to assert equality with the fixed
message "유효한 URL 형식이 아닙니다." so it matches the canonical
ProductLinkException.invalidFormat text and aligns with the other test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: ab4fe9b9-23ea-4b6e-b21a-2d93cf68a4a0
📒 Files selected for processing (4)
src/main/kotlin/com/depromeet/team3/product/domain/ProductLink.ktsrc/main/kotlin/com/depromeet/team3/product/domain/ProductLinkException.ktsrc/test/kotlin/com/depromeet/team3/product/domain/ProductLinkTest.ktsrc/test/kotlin/com/depromeet/team3/wishlist/controller/WishlistControllerIntegrationTest.kt
theo-s-park
reviewed
May 13, 2026
Comment on lines
+59
to
+60
| assertEquals("유효한 URL 형식이 아닙니다.", ex.message) | ||
| assertFalse(ex.message!!.contains("SHOULD_NOT_LEAK")) |
Collaborator
Author
There was a problem hiding this comment.
맞습니다 — 정확한 지적입니다. 바로 위 줄의 assertEquals("유효한 URL 형식이 아닙니다.", ex.message) 가 통과하면 assertFalse(contains) 는 항상 true 라 redundant 했어요. "원본이 안 박힌다" 의도를 명시적으로 박고 싶어서 두 줄로 두긴 했는데, equality 단언이 더 강한 보장이라 한 줄로 충분합니다.
d4002d8 에서 assertFalse 줄 제거했습니다. 동일 커밋에서 위쪽 URI 파싱 자체가 실패하는 raw… 테스트의 startsWith 도 assertEquals 로 일관화했고요 (CodeRabbit nitpick 함께 반영).
Collaborator
|
좋아 💯 |
- '유효한 URL 형식이 아닙니다' 케이스를 startsWith → assertEquals 로 정확 일치 단언. 메시지가 고정 문구라 equality 가 회귀 검출에 강하고 같은 파일 안 다른 단언들과 일관됨. - '예외 메시지에 원본 raw 는 포함되지 않는다' 테스트에서 assertEquals 뒤에 따라붙던 assertFalse(contains) 제거. equality 단언이 통과하면 contains 단언은 항상 true 라 redundant.
sevineleven
added a commit
that referenced
this pull request
May 16, 2026
- dev #118 이 추가한 통합 테스트가 요청 본문에 guestId 필드를 쓰는데 본 PR 의 Guest 도메인 제거 변경과 의미적으로 충돌해 dev 머지 후 CI 가 실패 - 같은 파일의 다른 케이스와 동일하게 userId 로 통일
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Situation
ProductLink.parse가 검증 실패 시IllegalArgumentException("…: $trimmed", e)로 원본 URL 을 메시지에 담고 있었다.GlobalExceptionHandler.handleIllegalArgument가e.message를 응답detail과 로그 양쪽에 그대로 박는 구조 → 쿼리스트링/fragment 에 섞인 토큰·세션이 외부로 새는 경로.safeLogString()주석이 이미 "쿼리스트링·fragment 에 토큰/세션이 섞일 수 있어 host+path 만 노출한다" 라고 명시 — 파일이 위험을 인식하면서도parse()만 inconsistency.Task
IllegalArgumentException일반 핸들러 정책에 묶이지 않게 분리.Action
ProductLinkException추가 (BaseException+HttpMappable, HTTP 400 +INVALID_INPUT).WishException,ProductExtractionException) 그대로 — companion object 정적 팩토리 메서드 패턴:blank()— 빈 입력invalidFormat(cause: Throwable)— URI 파싱 실패. 원본을 message 에 박지 않고 cause 로만 연결해 stack trace 에 남김.unsupportedScheme()— https 외 스킴ProductLink.parse의require·throw를 모두 새 예외로 전환.IllegalArgumentException경로 제거.ProductLinkTest) —ProductLinkException으로 단언 전환, 원본 raw 가 message 에 포함되지 않음 명시 단언 추가.WishlistControllerIntegrationTest) — 잘못된 형식 URL 요청 시 응답status·code·detailcontract 단언으로 회귀 가드.Result
IllegalArgumentException일반 핸들러 정책에 묶이지 않고, ProductLink 메시지 정책이 도메인 안에서 통제됨.GlobalExceptionHandler.handleIllegalArgument가 4xx 응답 detail 에e.message를 무방비로 박는 패턴 자체도 잠재 위험. 다른 4xx 응답 contract 와 얽혀 영향도 평가가 별도로 필요하므로 본 PR 스코프 밖. 필요하면 별도 이슈로.연관 이슈
Summary by CodeRabbit