Merged
Conversation
- 브랜드 필터링을 단일 → 다중 선택으로 변경 (brandIds) - 사이즈 필터링을 단일 → 다중 선택으로 변경 (clothingSizes) - 가격 필터링 시 할인가(discountPrice) 기준으로 적용되도록 수정 - 사이즈 REGEXP 패턴으로 정확한 값 매칭 처리 (S가 XS에 매칭되는 버그 수정) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
개요단일값 기반 상품 검색 필터(브랜드 ID, 의류 사이즈)를 다중값 리스트 기반 필터로 변경하고, 할인가 기반 가격 필터링 로직과 정규식 기반 사이즈 패턴 매칭을 도입하는 변경사항입니다. 변경 사항
추정 코드 리뷰 난이도🎯 3 (보통) | ⏱️ ~20분 이유: 네 개 파일에 걸친 일관된 패턴의 필터링 로직 변경이지만, 할인가 기반 가격 필터링(minPrice/maxPrice 조건 분기)과 정규식 기반 사이즈 매칭의 새로운 로직 밀도가 있으며, 리포지토리 계층에서 쿼리 조건의 복잡도가 증가했습니다. 관련 가능 PR
제안 라벨
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/ongil/backend/domain/product/service/ProductService.java (1)
339-360:⚠️ Potential issue | 🟠 Major가격 정렬 기준(
price)이 가격 필터 기준(discountPrice)과 불일치이번 PR에서 가격 필터를
discountPrice우선으로 변경했지만,PRICE_LOW/PRICE_HIGH정렬은 여전히 원래price필드 기준입니다. 결과적으로 필터에서 통과된 상품들이 사용자가 기대하는 실제 할인가 순서가 아닌 정가 순서로 정렬됩니다.예시:
- 상품 A: price=100,000, discountPrice=35,000 → 필터(30,000~40,000) 통과
- 상품 B: price=40,000, discountPrice=38,000 → 필터 통과
PRICE_LOW정렬 결과: B(40,000), A(100,000) — 그러나 실제 할인가는 A(35,000) < B(38,000)정렬도 유효가격(discountPrice > 0이면 discountPrice, 아니면 price)을 기준으로 사용하거나,
Product엔티티에effectivePrice계산 컬럼을 추가하는 방식으로 맞춰야 합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/ongil/backend/domain/product/service/ProductService.java` around lines 339 - 360, The PRICE_LOW/PRICE_HIGH branches in createSort(ProductSortType) still sort by the raw "price" field while filtering uses discountPrice; update sorting to use an effective-price field (discountPrice when >0 else price) so results match the filter. Modify createSort's PRICE_LOW and PRICE_HIGH cases to order by that effective price expression (or by a new computed property like effectivePrice on Product) instead of "price", keeping the tiebreakers (popularity desc, id asc) intact; ensure Product has the computed effectivePrice if you pick that approach and use its name in the Sort.
🧹 Nitpick comments (1)
src/main/java/com/ongil/backend/domain/product/repository/ProductRepository.java (1)
42-65:@Query+@EntityGraph+Page<>조합 시countQuery명시 권장Spring Data JPA는
Page<T>를 반환하는@Query에서 자동으로 COUNT 쿼리를 유도하는데,@EntityGraph가 함께 사용될 경우 auto-derived count 쿼리가 의도치 않은 JOIN을 포함하거나 성능 문제를 일으킬 수 있습니다.findAllByParentCategoryCondition(Line 68)도 동일합니다.♻️ countQuery 명시 예시
`@Query`(value = """ SELECT p FROM Product p WHERE (:targetIds IS NULL OR p.id IN :targetIds) ... - """) + """, + countQuery = """ + SELECT COUNT(p) FROM Product p + WHERE (:targetIds IS NULL OR p.id IN :targetIds) + AND (:categoryId IS NULL OR p.category.id = :categoryId) + AND (:brandIds IS NULL OR p.brand.id IN :brandIds) + AND (:minPrice IS NULL OR + (p.discountPrice IS NOT NULL AND p.discountPrice > 0 AND p.discountPrice >= :minPrice) + OR ((p.discountPrice IS NULL OR p.discountPrice = 0) AND p.price >= :minPrice)) + AND (:maxPrice IS NULL OR + (p.discountPrice IS NOT NULL AND p.discountPrice > 0 AND p.discountPrice <= :maxPrice) + OR ((p.discountPrice IS NULL OR p.discountPrice = 0) AND p.price <= :maxPrice)) + AND (:sizesPattern IS NULL OR FUNCTION('REGEXP_LIKE', p.sizes, :sizesPattern) = true) + AND p.onSale = true + """)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/ongil/backend/domain/product/repository/ProductRepository.java` around lines 42 - 65, The `@Query` with `@EntityGraph` returning Page<Product> can cause Spring Data JPA to derive a count query that includes unwanted joins and hurts performance; update the findAllByCondition (and similarly findAllByParentCategoryCondition) query declarations to supply an explicit countQuery that selects only the count of products (e.g., "SELECT COUNT(p) FROM Product p" with the same WHERE predicates but without fetching joins or attributePaths) so paging uses the lightweight count; keep the main JPQL (with attributePaths) as the value and add a tailored countQuery that omits entity-graph fetches to avoid extra JOINs and ensure correct counts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/com/ongil/backend/domain/product/dto/request/ProductSearchCondition.java`:
- Around line 23-24: The `@Schema` example on the sizes field in
ProductSearchCondition uses "[M, L]" which is not valid JSON; update the `@Schema`
annotation for the sizes field to provide a valid JSON array string (e.g. a
quoted JSON array with each element quoted) so Swagger displays it correctly and
ensure any inner quotes are properly escaped in the Java string literal.
- Around line 27-33: The buildSizesPattern() method concatenates raw
user-provided sizes into a regex and must be fixed to prevent regex injection:
in ProductSearchCondition, validate each entry of the sizes list against a
whitelist of allowed size tokens (e.g., your Size enum or a precompiled Set of
valid values) and discard/throw on invalid inputs, or alternatively escape each
value before joining (use Java's Pattern.quote for each size) inside
buildSizesPattern(); then build the pattern from the validated/escaped values so
the returned string is safe.
---
Outside diff comments:
In `@src/main/java/com/ongil/backend/domain/product/service/ProductService.java`:
- Around line 339-360: The PRICE_LOW/PRICE_HIGH branches in
createSort(ProductSortType) still sort by the raw "price" field while filtering
uses discountPrice; update sorting to use an effective-price field
(discountPrice when >0 else price) so results match the filter. Modify
createSort's PRICE_LOW and PRICE_HIGH cases to order by that effective price
expression (or by a new computed property like effectivePrice on Product)
instead of "price", keeping the tiebreakers (popularity desc, id asc) intact;
ensure Product has the computed effectivePrice if you pick that approach and use
its name in the Sort.
---
Nitpick comments:
In
`@src/main/java/com/ongil/backend/domain/product/repository/ProductRepository.java`:
- Around line 42-65: The `@Query` with `@EntityGraph` returning Page<Product> can
cause Spring Data JPA to derive a count query that includes unwanted joins and
hurts performance; update the findAllByCondition (and similarly
findAllByParentCategoryCondition) query declarations to supply an explicit
countQuery that selects only the count of products (e.g., "SELECT COUNT(p) FROM
Product p" with the same WHERE predicates but without fetching joins or
attributePaths) so paging uses the lightweight count; keep the main JPQL (with
attributePaths) as the value and add a tailored countQuery that omits
entity-graph fetches to avoid extra JOINs and ensure correct counts.
Comment on lines
+23
to
+24
| @Schema(description = "사이즈 목록 (다중 선택, 예: XS, S, M, L, XL)", example = "[M, L]") | ||
| private List<String> sizes; |
Contributor
There was a problem hiding this comment.
@Schema example 값이 유효하지 않은 JSON 형식
example = "[M, L]" 는 JSON 배열 형식이 아닙니다. Swagger UI에서 잘못된 예시로 표시됩니다.
📝 제안 수정
-@Schema(description = "사이즈 목록 (다중 선택, 예: XS, S, M, L, XL)", example = "[M, L]")
+@Schema(description = "사이즈 목록 (다중 선택, 예: XS, S, M, L, XL)", example = "[\"M\", \"L\"]")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/ongil/backend/domain/product/dto/request/ProductSearchCondition.java`
around lines 23 - 24, The `@Schema` example on the sizes field in
ProductSearchCondition uses "[M, L]" which is not valid JSON; update the `@Schema`
annotation for the sizes field to provide a valid JSON array string (e.g. a
quoted JSON array with each element quoted) so Swagger displays it correctly and
ensure any inner quotes are properly escaped in the Java string literal.
Comment on lines
+27
to
+33
| public String buildSizesPattern() { | ||
| if (sizes == null || sizes.isEmpty()) { | ||
| return null; | ||
| } | ||
| String sizeGroup = String.join("|", sizes); | ||
| return "(^|,)(" + sizeGroup + ")(,|$)"; | ||
| } |
Contributor
There was a problem hiding this comment.
buildSizesPattern() — 사용자 입력값이 정규식 패턴에 그대로 삽입되는 Regex Injection 취약점
sizes는 HTTP 쿼리 파라미터에서 직접 오는 사용자 입력값입니다. 현재 이 값들을 아무런 이스케이프 없이 정규식 문자열에 이어붙이고 있어서, 악의적인 사용자가 sizes=["M)|((.*)"]와 같은 값을 넘기면 패턴이 (^|,)(M)|((.*))(,|$)로 변형되어 사이즈 필터를 우회하고 모든 상품을 반환할 수 있습니다.
수정 방법은 두 가지입니다:
- 허용된 사이즈 값(enum 또는 정규식)으로 화이트리스트 검증 (권장)
- 각 사이즈 값에서 정규식 메타문자 제거
🛡️ 제안 수정 (화이트리스트 방식)
public String buildSizesPattern() {
if (sizes == null || sizes.isEmpty()) {
return null;
}
- String sizeGroup = String.join("|", sizes);
+ // 영숫자와 '+'만 허용하여 정규식 메타문자 삽입 차단
+ String sizeGroup = sizes.stream()
+ .map(s -> s.replaceAll("[^a-zA-Z0-9+]", ""))
+ .filter(s -> !s.isEmpty())
+ .collect(java.util.stream.Collectors.joining("|"));
+ if (sizeGroup.isEmpty()) {
+ return null;
+ }
return "(^|,)(" + sizeGroup + ")(,|$)";
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/ongil/backend/domain/product/dto/request/ProductSearchCondition.java`
around lines 27 - 33, The buildSizesPattern() method concatenates raw
user-provided sizes into a regex and must be fixed to prevent regex injection:
in ProductSearchCondition, validate each entry of the sizes list against a
whitelist of allowed size tokens (e.g., your Size enum or a precompiled Set of
valid values) and discard/throw on invalid inputs, or alternatively escape each
value before joining (use Java's Pattern.quote for each size) inside
buildSizesPattern(); then build the pattern from the validated/escaped values so
the returned string is safe.
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
1 participant
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.
🔍️ 작업 내용
상품 필터링 기능 개선
✨ 상세 설명
🛠️ 추후 리팩토링 및 고도화 계획
📸 스크린샷 (선택)
💬 리뷰 요구사항
Summary by CodeRabbit
릴리스 노트
새로운 기능