-
Notifications
You must be signed in to change notification settings - Fork 1
[fix] 배포 시 에러 로그 사라지는 버그 수정, 예외 디스코드 알림 범위 수정, 프론트 요구사항 반영 #133
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Warning Rate limit exceeded@KiSeungMin has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 1 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
Walkthrough배포 워크플로우와 Docker 이미지에서 호스트/컨테이너 로그 디렉토리를 생성·권한 설정하고 로그 상태 점검 단계를 추가했으며, 예외 처리기에서 Discord 알림의 조건·무시 로직을 도입·조정하고 logback의 ERROR 로그 롤링 시작 동작을 변경했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant Security as Auth
participant ExceptionHandler
participant Discord
Client->>Controller: 요청
Controller->>Auth: SecurityContext에서 인증된 userId 조회
Controller-->>ExceptionHandler: 예외 발생 시 전달
ExceptionHandler->>ExceptionHandler: shouldSkipDiscordNotification(path, ex, message) 판단
alt skip == false AND profile == develop
ExceptionHandler-->>Discord: sendToDiscord(ex, path, details)
else
Note over ExceptionHandler: Discord 알림 생략(로그로 기록)
end
ExceptionHandler-->>Client: ResponseEntity 반환
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/cd-develop.yml (1)
193-193: YAML 파일의 후행 공백 제거 필요Line 193에 불필요한 후행 공백이 있습니다.
- ls -la /var/log/wayble/ || echo "Log directory not found" - + ls -la /var/log/wayble/ || echo "Log directory not found" +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/cd-develop.yml(2 hunks)Dockerfile(1 hunks)src/main/java/com/wayble/server/common/exception/GlobalExceptionHandler.java(4 hunks)src/main/resources/logback-spring.xml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/cd-develop.yml
[error] 193-193: trailing spaces
(trailing-spaces)
🔇 Additional comments (12)
Dockerfile (2)
9-10: 로그 디렉토리 생성이 적절합니다!컨테이너 내부에 로그 디렉토리를 생성하고 적절한 권한을 설정한 것은 좋은 접근입니다. 이는 배포 시 로그가 사라지는 문제를 해결하는데 도움이 됩니다.
12-16: WORKDIR 설정으로 파일 구조가 개선되었습니다작업 디렉토리를
/app으로 설정하여 애플리케이션 파일들이 체계적으로 관리됩니다. JAR 파일이 이제/app/app.jar에 위치하게 되며, ENTRYPOINT는 변경 없이 올바르게 작동합니다.src/main/resources/logback-spring.xml (1)
37-37: cleanHistoryOnStart를 false로 변경한 것이 적절합니다!
cleanHistoryOnStart를false로 설정하여 애플리케이션 재시작 시 기존 에러 로그가 삭제되지 않도록 한 것은 배포 시 로그가 사라지는 버그를 해결하는 핵심 수정사항입니다.src/main/java/com/wayble/server/common/exception/GlobalExceptionHandler.java (5)
55-72: 메서드 시그니처 변경이 적절합니다
BindingResult파라미터를 제거하고ex.getBindingResult()를 직접 사용하도록 변경한 것이 좋습니다. 또한 validation 예외에 대한 Discord 알림을 제거한 것도 적절한 판단입니다.
143-168: Discord 알림 제외 로직이 체계적으로 구현되었습니다클라이언트 예외, 봇/크롤러 요청, 그리고 정적 리소스 요청 등을 Discord 알림에서 제외하는 로직이 잘 구조화되어 있습니다. 이는 불필요한 알림을 줄이는데 효과적입니다.
173-196: 보안 취약점 경로 필터링 추가는 좋은 방어책입니다
.env,config.php,/admin/등 일반적인 공격 시도 경로들을 필터링하여 Discord 알림 노이즈를 줄이는 것은 적절합니다.
201-218: 봇/크롤러 감지 로직이 적절합니다정적 리소스 요청이나 view resolution 실패 등 봇/크롤러가 발생시키는 일반적인 에러 패턴들을 잘 식별하고 있습니다.
223-240: 클라이언트 연결 종료 예외 처리가 적절합니다
ClientAbortException,BrokenPipeException및 타임아웃 관련 예외들을 Discord 알림에서 제외하는 것은 서버 측에서 제어할 수 없는 클라이언트 이슈들을 필터링하는 좋은 접근입니다..github/workflows/cd-develop.yml (4)
79-81: 호스트 로그 디렉토리 생성이 적절합니다배포 시 호스트 머신에
/var/log/wayble디렉토리를 생성하고 적절한 권한을 설정하는 것은 로그 영속성을 보장하는 좋은 접근입니다.
83-84: 볼륨 제외 옵션 추가가 중요합니다!
--volumes=false옵션을 추가하여 Docker 정리 시 볼륨을 보존하도록 한 것은 로그 데이터 손실을 방지하는 핵심 수정사항입니다.
150-150: 볼륨 마운트 설정이 올바릅니다호스트의
/var/log/wayble을 컨테이너의/app/logs에 마운트하여 로그가 컨테이너 재시작 후에도 유지되도록 설정한 것이 적절합니다.
191-200: 로그 상태 확인 로직이 유용합니다배포 후 로그 디렉토리와 에러 로그 파일의 상태를 확인하는 것은 디버깅과 모니터링에 도움이 됩니다.
| if (!env.acceptsProfiles(Profiles.of("local"))) { | ||
| log.info("현재 active 프로파일이 develop가 아니므로 Discord 알림을 보내지 않습니다."); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
프로파일 조건 검증 필요
현재 코드는 local 프로파일이 아닌 경우 Discord 알림을 보내지 않도록 되어 있으나, 로그 메시지는 여전히 "develop가 아니므로"라고 표시됩니다. 또한 PR 목적에서 언급된 대로 production 환경에서도 중요한 에러는 알림을 받아야 할 수 있습니다.
- if (!env.acceptsProfiles(Profiles.of("local"))) {
- log.info("현재 active 프로파일이 develop가 아니므로 Discord 알림을 보내지 않습니다.");
+ if (!env.acceptsProfiles(Profiles.of("develop", "production"))) {
+ log.info("현재 active 프로파일이 develop 또는 production이 아니므로 Discord 알림을 보내지 않습니다.");
return;
}🤖 Prompt for AI Agents
In src/main/java/com/wayble/server/common/exception/GlobalExceptionHandler.java
around lines 100-103, the profile check is inverted and the log message is
wrong: change the condition to skip Discord notifications only when the active
profile is "local" (remove the negation), update the log message to reference
"local" instead of "develop", and ensure production and other non-local profiles
will proceed to send important error alerts; keep the check clear (e.g.,
env.acceptsProfiles(Profiles.of("local")) -> log and return).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🔭 Outside diff range comments (1)
src/main/java/com/wayble/server/common/exception/GlobalExceptionHandler.java (1)
128-137: Discord 전송 결과/실패 로깅 보강 및 민감정보 재노출 방지성공 시 payload 전체를 INFO로 남기면 예외 메시지/경로가 이중 노출될 수 있습니다. 성공은 경량 메타만, 실패는 스택트레이스를 함께 남겨 원인 파악을 쉽게 하세요.
- log.info("send to Discord webhook: {} complete", payload); + log.info("send to Discord webhook complete: path={}, status={}", + ((ServletWebRequest) request).getRequest().getRequestURI(), status); } catch (Exception e){ - log.error(e.getMessage()); + log.error("Discord webhook 전송 실패 - path={}, status={}, url={}", + ((ServletWebRequest) request).getRequest().getRequestURI(), status, discordWebhookUrl, e); }
♻️ Duplicate comments (1)
src/main/java/com/wayble/server/common/exception/GlobalExceptionHandler.java (1)
100-103: 프로파일 게이팅 로직 재검토 — develop 전용이면 운영/스테이징 알림 누락 가능현재 조건은 develop에서만 Discord 알림을 보내고, 운영 포함 그 외 모든 프로파일에서는 알림을 막습니다. PR 목적(클라이언트 성격의 예외는 제외)에는 부합하지만, 운영 환경의 중요 서버 에러까지 차단될 수 있습니다. 팀 정책에 따라 로컬/테스트에서만 제외하고 나머지(개발/운영/스테이징)는 허용하는 편이 일반적입니다.
다음과 같이 “local/test에서만 제외”로 전환하는 것을 제안합니다:
- if (!env.acceptsProfiles(Profiles.of("develop"))) { - log.info("현재 active 프로파일이 develop가 아니므로 Discord 알림을 보내지 않습니다."); + if (env.acceptsProfiles(Profiles.of("local", "test"))) { + log.info("현재 active 프로파일이 local/test이므로 Discord 알림을 보내지 않습니다."); return; }프로파일 명칭(dev vs develop, prod vs production 등)을 저장소에서 한 번 확인해 주세요. 아래 스크립트로 프로파일 파일/설정과 코드 사용처를 점검할 수 있습니다.
#!/bin/bash # 프로파일 파일/명칭, 사용처 점검 echo "== application* 프로퍼티/설정 파일 확인 ==" fd -HI --type f --regex 'application(-[a-zA-Z0-9_-]+)?\.(ya?ml|properties)$' | sort echo echo "== spring.profiles.* 키 사용 라인 ==" rg -n -S 'spring\.profiles\.(active|group|include)' rg -n -S 'spring\.config\.activate\.on-profile' echo echo "== 코드 내 Profiles.of 사용처 ==" rg -n -S 'Profiles\.of\(' echo echo "== 코드/설정 내 프로파일 키워드(대략 검색) ==" rg -n -S '\b(dev|develop|prod|production|stage|staging|local|test)\b'
🧹 Nitpick comments (3)
src/main/java/com/wayble/server/explore/controller/WaybleZoneRecommendController.java (1)
9-9: 필요한 import 추가.Authentication 클래스 import가 필요합니다.
+import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContextHolder;src/main/java/com/wayble/server/common/exception/GlobalExceptionHandler.java (2)
64-65: WARN 로그에 필드명 포함으로 트러블슈팅 가속첫 번째 필드 에러명을 함께 남기면 원인 파악이 빨라집니다.
아래 diff로 로그 포맷을 보강해 주세요:
- log.warn("Validation Exception - Method: {}, Path: {}, Message: {}, Location: {}", - method, path, message, errorLocation); + log.warn("Validation Exception - Method: {}, Path: {}, Field: {}, Message: {}, Location: {}", + method, path, firstField, message, errorLocation);변경 라인 외에 필요한 보조 코드(라인 62 인근)에 다음을 추가해 주세요:
// 보조 코드: 라인 62 인근에 배치 String firstField = (ex.getBindingResult() != null && ex.getBindingResult().getFieldError() != null) ? ex.getBindingResult().getFieldError().getField() : "-";
112-121: Discord로 전송하는 메시지 길이/민감정보 노출 최소화예외 메시지는 길거나 민감정보(쿼리 파라미터, 토큰 등)를 포함할 수 있습니다. 트렁케이션(예: 500자)과 기본값을 두는 것이 안전합니다.
아래와 같이 메시지 변수를 도입하고 임베드에 사용해 주세요:
- DiscordWebhookPayload.Embed embed = new DiscordWebhookPayload.Embed( + String messageForDiscord = ex.getMessage() != null ? ex.getMessage() : "(no message)"; + if (messageForDiscord.length() > 500) { + messageForDiscord = messageForDiscord.substring(0, 500) + "..."; + } + DiscordWebhookPayload.Embed embed = new DiscordWebhookPayload.Embed( "🚨 서버 에러 발생", - "```" + ex.getMessage() + "```", + "```" + messageForDiscord + "```", timestamp, List.of( new DiscordWebhookPayload.Embed.Field("URL", path, false), new DiscordWebhookPayload.Embed.Field("Status", status.toString(), true), new DiscordWebhookPayload.Embed.Field("Time", timestamp, true), new DiscordWebhookPayload.Embed.Field("Exception", ex.getClass().getSimpleName(), true) ) );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/wayble/server/common/exception/GlobalExceptionHandler.java(4 hunks)src/main/java/com/wayble/server/explore/controller/WaybleZoneRecommendController.java(2 hunks)src/main/java/com/wayble/server/explore/dto/recommend/WaybleZoneRecommendConditionDto.java(1 hunks)src/main/java/com/wayble/server/user/controller/UserPlaceController.java(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/main/java/com/wayble/server/explore/controller/WaybleZoneRecommendController.java (3)
src/main/java/com/wayble/server/common/config/security/jwt/JwtAuthentication.java (2)
Override(18-18)JwtAuthentication(11-15)src/main/java/com/wayble/server/user/controller/UserController.java (1)
PatchMapping(71-90)src/main/java/com/wayble/server/common/config/security/jwt/JwtTokenProvider.java (1)
getUserId(53-64)
src/main/java/com/wayble/server/user/controller/UserPlaceController.java (3)
src/main/java/com/wayble/server/user/service/UserPlaceService.java (1)
Service(22-96)src/main/java/com/wayble/server/user/entity/UserPlace.java (1)
Entity(11-29)src/main/java/com/wayble/server/user/repository/UserPlaceRepository.java (1)
UserPlaceRepository(8-10)
src/main/java/com/wayble/server/common/exception/GlobalExceptionHandler.java (1)
src/main/java/com/wayble/server/common/MainController.java (1)
RestController(9-36)
src/main/java/com/wayble/server/explore/dto/recommend/WaybleZoneRecommendConditionDto.java (1)
src/main/java/com/wayble/server/explore/dto/search/request/WaybleZoneSearchConditionDto.java (1)
Builder(10-30)
🔇 Additional comments (5)
src/main/java/com/wayble/server/explore/dto/recommend/WaybleZoneRecommendConditionDto.java (1)
17-18: @NotNull 어노테이션은 제거되지 않았습니다.
WaybleZoneRecommendConditionDto를 보면latitude와longitude필드 모두@NotNull으로 선언되어 있어 null 허용이 아닙니다. 해당 지적은 적용되지 않으니 무시해 주세요.Likely an incorrect or invalid review comment.
src/main/java/com/wayble/server/user/controller/UserPlaceController.java (2)
21-21: API 경로 변경이 적절합니다.RESTful 설계 원칙에 따라 사용자별 리소스에 대한 경로에서 인증된 사용자 컨텍스트를 활용하는 방식으로 변경된 것이 좋습니다.
/api/v1/users/{userId}/places에서/api/v1/users/places로 변경하여 보안성과 일관성이 향상되었습니다.
35-37: 메서드 시그니처 개선.@PathVariable userId를 제거하고 보안 컨텍스트에서 userId를 추출하도록 변경한 것이 보안상 더 안전합니다. 이제 클라이언트가 다른 사용자의 ID를 임의로 전달할 수 없게 되었습니다.
Also applies to: 53-54
src/main/java/com/wayble/server/common/exception/GlobalExceptionHandler.java (2)
41-41: 의미 없는 변경해당 라인은 공백 추가 외 실질적인 변화가 없어 코멘트를 생략합니다.
105-110: 노이즈 필터링(Discord 알림 제외) 로직 추가 적절합니다봇/정적자원/일부 무시 예외를 중앙에서 걸러 노이즈를 줄이는 접근이 명확합니다. 실제 운영 중에도 유지보수하기 쉬운 구조입니다.
| @ExceptionHandler(MethodArgumentNotValidException.class) | ||
| public ResponseEntity<CommonResponse> handleValidException(MethodArgumentNotValidException ex, WebRequest request) { | ||
| String message = ex.getBindingResult().getAllErrors().get(0).getDefaultMessage(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
검증 예외 메시지 추출 시 안전성 보강 (빈 에러/NPE 방지)
getAllErrors().get(0)는 드물게 비어 있을 수 있어 IndexOutOfBoundsException 가능성이 있습니다. 메시지가 null/blank인 경우도 대비해 기본 메시지를 두는 것이 안전합니다.
다음과 같이 방어 로직을 적용해 주세요:
- String message = ex.getBindingResult().getAllErrors().get(0).getDefaultMessage();
+ String message = "요청 검증에 실패했습니다.";
+ if (ex.getBindingResult() != null) {
+ if (!ex.getBindingResult().getAllErrors().isEmpty()) {
+ String m = ex.getBindingResult().getAllErrors().get(0).getDefaultMessage();
+ if (m != null && !m.isBlank()) {
+ message = m;
+ }
+ } else if (ex.getMessage() != null && !ex.getMessage().isBlank()) {
+ message = ex.getMessage();
+ }
+ } else if (ex.getMessage() != null && !ex.getMessage().isBlank()) {
+ message = ex.getMessage();
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @ExceptionHandler(MethodArgumentNotValidException.class) | |
| public ResponseEntity<CommonResponse> handleValidException(MethodArgumentNotValidException ex, WebRequest request) { | |
| String message = ex.getBindingResult().getAllErrors().get(0).getDefaultMessage(); | |
| @ExceptionHandler(MethodArgumentNotValidException.class) | |
| public ResponseEntity<CommonResponse> handleValidException(MethodArgumentNotValidException ex, WebRequest request) { | |
| // 기본 메시지 설정 | |
| String message = "요청 검증에 실패했습니다."; | |
| if (ex.getBindingResult() != null) { | |
| if (!ex.getBindingResult().getAllErrors().isEmpty()) { | |
| String m = ex.getBindingResult().getAllErrors().get(0).getDefaultMessage(); | |
| if (m != null && !m.isBlank()) { | |
| message = m; | |
| } | |
| } else if (ex.getMessage() != null && !ex.getMessage().isBlank()) { | |
| message = ex.getMessage(); | |
| } | |
| } else if (ex.getMessage() != null && !ex.getMessage().isBlank()) { | |
| message = ex.getMessage(); | |
| } | |
| // 이후 로직 계속 (예: CommonResponse 생성 및 반환) | |
| CommonResponse body = CommonResponse.error(message); | |
| return ResponseEntity.badRequest().body(body); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/wayble/server/common/exception/GlobalExceptionHandler.java
around lines 55-57, the current use of
ex.getBindingResult().getAllErrors().get(0).getDefaultMessage() can throw
IndexOutOfBoundsException when the error list is empty and may return
null/blank; update the handler to defensively retrieve the first error only
after checking that getAllErrors() is non-null and non-empty, extract
getDefaultMessage() safely (treating null/blank as absent), and fall back to a
sensible default message such as "Invalid request" (or a constant) if no usable
message exists; ensure no NPEs by null-checking the BindingResult and
DefaultMessage and consider trimming the message before returning it in the
ResponseEntity.
| /** | ||
| * 무시할 경로인지 확인 | ||
| */ | ||
| private boolean isIgnoredPath(String path) { | ||
| String[] ignoredPaths = { | ||
| "/favicon.ico", | ||
| "/index.html", | ||
| "/robots.txt", | ||
| "/sitemap.xml", | ||
| "/apple-touch-icon", | ||
| "/.well-known/", | ||
| "/wp-admin/", | ||
| "/admin/", | ||
| "/phpmyadmin/", | ||
| "/xmlrpc.php", | ||
| "/.env", | ||
| "/config.php" | ||
| }; | ||
|
|
||
| for (String ignoredPath : ignoredPaths) { | ||
| if (path.contains(ignoredPath)) { | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
isIgnoredPath: 대소문자/널 안전성 보강
요청 경로가 null이거나 대문자 포함 시 매칭 누락 가능성이 있습니다. 소문자 변환 후 비교하도록 안전성 보강을 권장합니다.
- private boolean isIgnoredPath(String path) {
- String[] ignoredPaths = {
+ private boolean isIgnoredPath(String path) {
+ if (path == null) return false;
+ String lowerPath = path.toLowerCase();
+ String[] ignoredPaths = {
"/favicon.ico",
"/index.html",
"/robots.txt",
"/sitemap.xml",
"/apple-touch-icon",
"/.well-known/",
"/wp-admin/",
"/admin/",
"/phpmyadmin/",
"/xmlrpc.php",
"/.env",
"/config.php"
};
- for (String ignoredPath : ignoredPaths) {
- if (path.contains(ignoredPath)) {
+ for (String ignoredPath : ignoredPaths) {
+ if (lowerPath.contains(ignoredPath)) {
return true;
}
}
return false;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * 무시할 경로인지 확인 | |
| */ | |
| private boolean isIgnoredPath(String path) { | |
| String[] ignoredPaths = { | |
| "/favicon.ico", | |
| "/index.html", | |
| "/robots.txt", | |
| "/sitemap.xml", | |
| "/apple-touch-icon", | |
| "/.well-known/", | |
| "/wp-admin/", | |
| "/admin/", | |
| "/phpmyadmin/", | |
| "/xmlrpc.php", | |
| "/.env", | |
| "/config.php" | |
| }; | |
| for (String ignoredPath : ignoredPaths) { | |
| if (path.contains(ignoredPath)) { | |
| return true; | |
| } | |
| } | |
| return false; | |
| } | |
| private boolean isIgnoredPath(String path) { | |
| if (path == null) return false; | |
| String lowerPath = path.toLowerCase(); | |
| String[] ignoredPaths = { | |
| "/favicon.ico", | |
| "/index.html", | |
| "/robots.txt", | |
| "/sitemap.xml", | |
| "/apple-touch-icon", | |
| "/.well-known/", | |
| "/wp-admin/", | |
| "/admin/", | |
| "/phpmyadmin/", | |
| "/xmlrpc.php", | |
| "/.env", | |
| "/config.php" | |
| }; | |
| for (String ignoredPath : ignoredPaths) { | |
| if (lowerPath.contains(ignoredPath)) { | |
| return true; | |
| } | |
| } | |
| return false; | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/wayble/server/common/exception/GlobalExceptionHandler.java
around lines 170 to 197, the isIgnoredPath method is not null-safe and is
case-sensitive; update it to first handle a null path (return false immediately)
and normalize the incoming path to lowercase (and optionally trim) before
checking contains against the ignoredPaths entries (which can remain lowercase),
so comparisons succeed regardless of input case and avoid NPEs.
| /** | ||
| * 봇이나 크롤러 요청인지 확인 | ||
| */ | ||
| private boolean isBotOrCrawlerRequest(String message) { | ||
| if (message == null) return false; | ||
|
|
||
| String[] botIndicators = { | ||
| "No static resource", | ||
| "Could not resolve view", | ||
| "favicon", | ||
| "robots.txt" | ||
| }; | ||
|
|
||
| for (String indicator : botIndicators) { | ||
| if (message.contains(indicator)) { | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
봇/크롤러 감지: 대소문자 불일치 보완
메시지 비교를 소문자 기준으로 수행해 탐지 누락을 줄이는 것을 권장합니다.
- private boolean isBotOrCrawlerRequest(String message) {
- if (message == null) return false;
-
- String[] botIndicators = {
- "No static resource",
- "Could not resolve view",
- "favicon",
- "robots.txt"
- };
-
- for (String indicator : botIndicators) {
- if (message.contains(indicator)) {
- return true;
- }
- }
-
- return false;
- }
+ private boolean isBotOrCrawlerRequest(String message) {
+ if (message == null) return false;
+
+ String msg = message.toLowerCase();
+ String[] botIndicators = {
+ "no static resource",
+ "could not resolve view",
+ "favicon",
+ "robots.txt"
+ };
+
+ for (String indicator : botIndicators) {
+ if (msg.contains(indicator)) {
+ return true;
+ }
+ }
+
+ return false;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * 봇이나 크롤러 요청인지 확인 | |
| */ | |
| private boolean isBotOrCrawlerRequest(String message) { | |
| if (message == null) return false; | |
| String[] botIndicators = { | |
| "No static resource", | |
| "Could not resolve view", | |
| "favicon", | |
| "robots.txt" | |
| }; | |
| for (String indicator : botIndicators) { | |
| if (message.contains(indicator)) { | |
| return true; | |
| } | |
| } | |
| return false; | |
| } | |
| private boolean isBotOrCrawlerRequest(String message) { | |
| if (message == null) return false; | |
| String msg = message.toLowerCase(); | |
| String[] botIndicators = { | |
| "no static resource", | |
| "could not resolve view", | |
| "favicon", | |
| "robots.txt" | |
| }; | |
| for (String indicator : botIndicators) { | |
| if (msg.contains(indicator)) { | |
| return true; | |
| } | |
| } | |
| return false; | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/wayble/server/common/exception/GlobalExceptionHandler.java
around lines 198 to 219, the bot/crawler detection does a case-sensitive
contains check which can miss matches; normalize the input and indicators to a
consistent case (e.g., call message = message.toLowerCase(Locale.ROOT) and
compare against lower-cased indicator strings or call
message.toLowerCase(Locale.ROOT).contains(indicator.toLowerCase(Locale.ROOT)))
so all comparisons are case-insensitive and deterministic.
| /** | ||
| * 무시할 예외인지 확인 | ||
| */ | ||
| private boolean isIgnoredException(String exceptionName, String message) { | ||
| // 클라이언트 연결 종료 관련 | ||
| if ("ClientAbortException".equals(exceptionName) || | ||
| "BrokenPipeException".equals(exceptionName)) { | ||
| return true; | ||
| } | ||
|
|
||
| // 타임아웃 관련 (너무 빈번한 경우) | ||
| if (message != null && ( | ||
| message.contains("Connection timed out") || | ||
| message.contains("Read timed out") || | ||
| message.contains("Connection reset") | ||
| )) { | ||
| return true; | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
무시 예외 케이스 보강: broken pipe 등 메시지 기반 추가, 대소문자 보완
실무에서 “Broken pipe”, “Connection reset by peer” 형태의 메시지가 자주 발생합니다. 메시지 기반 케이스를 소문자로 통일해 보강하세요.
- private boolean isIgnoredException(String exceptionName, String message) {
+ private boolean isIgnoredException(String exceptionName, String message) {
// 클라이언트 연결 종료 관련
if ("ClientAbortException".equals(exceptionName) ||
"BrokenPipeException".equals(exceptionName)) {
return true;
}
// 타임아웃 관련 (너무 빈번한 경우)
- if (message != null && (
- message.contains("Connection timed out") ||
- message.contains("Read timed out") ||
- message.contains("Connection reset")
- )) {
- return true;
- }
+ if (message != null) {
+ String msg = message.toLowerCase();
+ if (msg.contains("connection timed out")
+ || msg.contains("read timed out")
+ || msg.contains("connection reset")
+ || msg.contains("connection reset by peer")
+ || msg.contains("broken pipe")) {
+ return true;
+ }
+ }
return false;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * 무시할 예외인지 확인 | |
| */ | |
| private boolean isIgnoredException(String exceptionName, String message) { | |
| // 클라이언트 연결 종료 관련 | |
| if ("ClientAbortException".equals(exceptionName) || | |
| "BrokenPipeException".equals(exceptionName)) { | |
| return true; | |
| } | |
| // 타임아웃 관련 (너무 빈번한 경우) | |
| if (message != null && ( | |
| message.contains("Connection timed out") || | |
| message.contains("Read timed out") || | |
| message.contains("Connection reset") | |
| )) { | |
| return true; | |
| } | |
| return false; | |
| } | |
| /** | |
| * 무시할 예외인지 확인 | |
| */ | |
| private boolean isIgnoredException(String exceptionName, String message) { | |
| // 클라이언트 연결 종료 관련 | |
| if ("ClientAbortException".equals(exceptionName) || | |
| "BrokenPipeException".equals(exceptionName)) { | |
| return true; | |
| } | |
| // 타임아웃 및 연결 종료 메시지 처리 (너무 빈번한 경우) | |
| if (message != null) { | |
| String msg = message.toLowerCase(); | |
| if (msg.contains("connection timed out") | |
| || msg.contains("read timed out") | |
| || msg.contains("connection reset") | |
| || msg.contains("connection reset by peer") | |
| || msg.contains("broken pipe")) { | |
| return true; | |
| } | |
| } | |
| return false; | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/wayble/server/common/exception/GlobalExceptionHandler.java
around lines 220 to 241, the ignored-exception checks should be made
case-insensitive and extended to match common message variants (e.g., "broken
pipe", "connection reset by peer"). Change exceptionName comparisons to use
case-insensitive checks (equalsIgnoreCase) and normalize message to lower-case
(after null check) then check contains(...) for "broken pipe", "connection reset
by peer", "connection timed out", "read timed out", and "connection reset" so
all variants are caught reliably.
| @Valid @ModelAttribute WaybleZoneRecommendConditionDto conditionDto, | ||
| @RequestParam(name = "size", defaultValue = "1") int size) { | ||
|
|
||
| Long userId = (Long) SecurityContextHolder.getContext().getAuthentication().getPrincipal(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
보안 컨텍스트에서 userId 추출 시 타입 검증이 누락되었습니다.
SecurityContextHolder에서 Principal을 Long으로 직접 캐스팅하고 있는데, 타입 안전성을 위해 instanceof 검증을 추가해야 합니다. UserController.java의 updateUserInfo 메서드에서는 이런 검증을 수행하고 있습니다.
다음과 같이 타입 검증을 추가하세요:
- Long userId = (Long) SecurityContextHolder.getContext().getAuthentication().getPrincipal();
+ Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
+ if (!(authentication.getPrincipal() instanceof Long)) {
+ throw new ApplicationException(UserErrorCase.FORBIDDEN);
+ }
+ Long userId = (Long) authentication.getPrincipal();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Long userId = (Long) SecurityContextHolder.getContext().getAuthentication().getPrincipal(); | |
| Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); | |
| if (!(authentication.getPrincipal() instanceof Long)) { | |
| throw new ApplicationException(UserErrorCase.FORBIDDEN); | |
| } | |
| Long userId = (Long) authentication.getPrincipal(); |
🤖 Prompt for AI Agents
In
src/main/java/com/wayble/server/explore/controller/WaybleZoneRecommendController.java
around line 28, the code directly casts
SecurityContextHolder.getContext().getAuthentication().getPrincipal() to Long;
add a type check before casting (e.g., if (principal instanceof Long) { userId =
(Long) principal; } else if (principal instanceof UserDetails) { extract the id
from the UserDetails implementation } else { throw an appropriate
access/authorization exception or return an error response) } so the principal
is validated and handled safely instead of assuming a Long.
| throw new ApplicationException(UserErrorCase.FORBIDDEN); | ||
| } | ||
|
|
||
| Long userId = (Long) SecurityContextHolder.getContext().getAuthentication().getPrincipal(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
타입 안전성을 위한 instanceof 검증이 누락되었습니다.
SecurityContextHolder에서 Principal을 Long으로 직접 캐스팅하고 있는데, UserController.java의 패턴을 따라 타입 검증을 추가해야 합니다.
- Long userId = (Long) SecurityContextHolder.getContext().getAuthentication().getPrincipal();
+ Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
+ if (!(authentication.getPrincipal() instanceof Long)) {
+ throw new ApplicationException(UserErrorCase.FORBIDDEN);
+ }
+ Long userId = (Long) authentication.getPrincipal();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Long userId = (Long) SecurityContextHolder.getContext().getAuthentication().getPrincipal(); | |
| Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); | |
| if (!(authentication.getPrincipal() instanceof Long)) { | |
| throw new ApplicationException(UserErrorCase.FORBIDDEN); | |
| } | |
| Long userId = (Long) authentication.getPrincipal(); |
🤖 Prompt for AI Agents
In src/main/java/com/wayble/server/user/controller/UserPlaceController.java
around line 38, the code directly casts
SecurityContextHolder.getContext().getAuthentication().getPrincipal() to Long
without checking the runtime type; update it to first check that the principal
is an instance of Long (or extract the expected user id via the same safe
pattern used in UserController.java), then cast; if the check fails, handle it
by throwing an appropriate authentication/illegal state exception or returning
an unauthorized response so the method remains type-safe.
| if (!userId.equals(tokenUserId)) { | ||
| throw new ApplicationException(UserErrorCase.FORBIDDEN); | ||
| } | ||
| Long userId = (Long) SecurityContextHolder.getContext().getAuthentication().getPrincipal(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
타입 안전성을 위한 instanceof 검증이 누락되었습니다.
동일하게 이 메서드에서도 타입 검증이 필요합니다.
- Long userId = (Long) SecurityContextHolder.getContext().getAuthentication().getPrincipal();
+ Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
+ if (!(authentication.getPrincipal() instanceof Long)) {
+ throw new ApplicationException(UserErrorCase.FORBIDDEN);
+ }
+ Long userId = (Long) authentication.getPrincipal();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Long userId = (Long) SecurityContextHolder.getContext().getAuthentication().getPrincipal(); | |
| Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); | |
| if (!(authentication.getPrincipal() instanceof Long)) { | |
| throw new ApplicationException(UserErrorCase.FORBIDDEN); | |
| } | |
| Long userId = (Long) authentication.getPrincipal(); |
🤖 Prompt for AI Agents
In src/main/java/com/wayble/server/user/controller/UserPlaceController.java
around line 55, the code casts
SecurityContextHolder.getContext().getAuthentication().getPrincipal() directly
to Long without an instanceof check; add a type safety guard: first check if
getPrincipal() instanceof Long before casting, and handle the else case (throw
an appropriate exception such as BadCredentialsException or return a 401/400
response), or extract the id from a known UserDetails implementation if
applicable; ensure the code only uses the Long value after the instanceof check
to avoid ClassCastException.
✔️ 연관 이슈
📝 작업 내용
Summary by CodeRabbit