Conversation
Test Results66 tests 66 ✅ 2s ⏱️ Results for commit a4cbf47. ♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
backend/src/main/java/moadong/media/util/S3Config.java (2)
3-3: 사용하지 않는 임포트 제거 필요
com.google.storage.v2.ListBucketsResponse임포트가 사용되지 않고 있습니다. 불필요한 임포트는 제거하는 것이 좋습니다.-import com.google.storage.v2.ListBucketsResponse;
28-37: 구현이 잘 되어 있으나 R2 설정에 대한 주석이 더 명확하면 좋겠습니다.S3Client 빈 설정이 잘 되어 있으며, Cloudflare R2 환경에 맞게 path-style access를 사용하고 있습니다. 주석이 적절히 추가되어 있으나, 다른 개발자들을 위해 R2와 AWS S3의 차이점에 대한 참고 링크를 추가하는 것이 도움이 될 수 있습니다.
.credentialsProvider(StaticCredentialsProvider.create(AwsBasicCredentials.create(accessKey, secretKey))) .endpointOverride(URI.create(endpoint)) .region(Region.US_EAST_1) // Region은 아무거나 넣어도 되지만 필수 .serviceConfiguration(S3Configuration.builder() - .pathStyleAccessEnabled(true) // Cloudflare R2에서 필수 + .pathStyleAccessEnabled(true) // Cloudflare R2에서 필수 (참고: https://developers.cloudflare.com/r2/api/s3/api/) .build()) .build();backend/src/main/java/moadong/media/controller/ClubImageController.java (1)
29-33: 롬복 RequiredArgsConstructor 대체 구현이 적절하나 필드 순서 개선 필요롬복
@RequiredArgsConstructor대신 명시적 생성자를 사용하여 명확하게"googleDrive"구현체를 주입받도록 처리한 점은 좋습니다. 하지만 일반적으로 필드 선언이 생성자보다 앞에 위치하는 것이 자바 코드 컨벤션에 맞습니다.public class ClubImageController { + private final ClubImageService clubImageService; + public ClubImageController(@Qualifier("googleDrive") ClubImageService clubImageService) { this.clubImageService = clubImageService; } - private final ClubImageService clubImageService;backend/src/main/java/moadong/media/controller/CloudflareImageController.java (1)
22-27: Tag 이름 중복 가능성 고려 필요
ClubImageController와 동일한 태그 이름"ClubImage"를 사용하고 있어 Swagger 문서에서 혼란을 줄 수 있습니다. 별도의 태그 이름(예:"CloudflareClubImage")을 사용하거나 태그 설명에 Cloudflare 저장소 관련 API임을 명시하는 것이 좋습니다.@RestController @RequestMapping("/api/club/cloudflare") -@Tag(name = "ClubImage", description = "클럽 이미지 관련 API") +@Tag(name = "CloudflareClubImage", description = "Cloudflare를 이용한 클럽 이미지 관련 API") @PreAuthorize("isAuthenticated()") @SecurityRequirement(name = "BearerAuth")backend/src/main/java/moadong/media/service/CloudflareImageService.java (2)
25-39: 서비스 설정이 잘 되어 있으나 빈 주입 방식 개선 고려서비스 Bean 설정이 잘 되어 있으며 필요한 의존성들이 적절히 주입되고 있습니다. 다만
@RequiredArgsConstructor를 사용하기보다는 명시적인 생성자를 사용하여 필수 의존성을 더 명확하게 표현하는 것을 고려해볼 수 있습니다. 또한MAX_FEED_COUNT와 같은 상수 필드명은 camelCase보다MAX_FEED_COUNT와 같이 UPPER_SNAKE_CASE를 사용하는 것이 자바 코딩 컨벤션에 더 맞습니다.@Service("cloudflare") -@RequiredArgsConstructor public class CloudflareImageService implements ClubImageService{ private final ClubRepository clubRepository; private final S3Client s3Client; @Value("${server.feed.max-count}") private int MAX_FEED_COUNT; @Value("${cloud.aws.s3.bucket}") private String bucketName; @Value("${cloud.aws.s3.view-endpoint}") private String viewEndpoint; + + public CloudflareImageService(ClubRepository clubRepository, S3Client s3Client) { + this.clubRepository = clubRepository; + this.s3Client = s3Client; + }
106-117: 파일 삭제 로직에 주석으로 URL 형식을 설명한 점이 좋습니다.S3에서 파일 삭제 로직이 적절하게 구현되어 있으며, URL 형식에 대한 주석으로 키 추출 로직을 설명한 점이 좋습니다. 다만, URL 파싱이 더 견고하도록 정규식이나 URL 파싱 라이브러리를 사용하는 것을 고려해볼 수 있습니다.
public void deleteFile(Club club, String filePath) { // https://pub-8655aea549d544239ad12d0385aa98aa.r2.dev/{key} -> {key} - String key = filePath.substring(viewEndpoint.length()+1); + String key = filePath.replace(viewEndpoint + "/", ""); DeleteObjectRequest deleteRequest = DeleteObjectRequest.builder() .bucket(bucketName) .key(key) .build(); s3Client.deleteObject(deleteRequest); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
backend/build.gradle(1 hunks)backend/src/main/java/moadong/media/controller/CloudflareImageController.java(1 hunks)backend/src/main/java/moadong/media/controller/ClubImageController.java(2 hunks)backend/src/main/java/moadong/media/service/CloudflareImageService.java(1 hunks)backend/src/main/java/moadong/media/service/GcsClubImageService.java(2 hunks)backend/src/main/java/moadong/media/service/GoogleDriveClubImageService.java(3 hunks)backend/src/main/java/moadong/media/util/ClubImageUtil.java(1 hunks)backend/src/main/java/moadong/media/util/S3Config.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
backend/src/main/java/moadong/media/service/GcsClubImageService.java (1)
backend/src/main/java/moadong/media/util/ClubImageUtil.java (1)
ClubImageUtil(6-12)
backend/src/main/java/moadong/media/service/GoogleDriveClubImageService.java (1)
backend/src/main/java/moadong/media/util/ClubImageUtil.java (1)
ClubImageUtil(6-12)
backend/src/main/java/moadong/media/controller/CloudflareImageController.java (1)
backend/src/main/java/moadong/media/controller/ClubImageController.java (1)
RestController(22-62)
🔇 Additional comments (18)
backend/build.gradle (1)
55-59: AWS S3 의존성을 추가하여 Cloudflare R2와의 호환성을 확보했습니다빌드 구성에 S3 관련 의존성을 적절하게 추가하였습니다. AWS BOM(Bill of Materials)을 사용하여 일관된 버전 관리를 하는 것은 좋은 방식입니다.
backend/src/main/java/moadong/media/service/GcsClubImageService.java (2)
3-3: 가져오기 문 업데이트 확인
containsKorean에서containsKoreanOrBlank로 가져오기 문을 올바르게 업데이트했습니다.
143-143: 파일명 검증 로직 개선됨한글 포함 여부 확인에서 한글 또는 공백 포함 여부 확인으로 조건을 개선하였습니다. 이로써 공백이 포함된 파일명도 랜덤 문자열로 대체되어 파일명 관련 문제를 방지할 수 있습니다.
backend/src/main/java/moadong/media/service/GoogleDriveClubImageService.java (4)
3-3: 가져오기 문 업데이트 확인
containsKorean에서containsKoreanOrBlank로 가져오기 문을 올바르게 업데이트했습니다.
25-25: 서비스 한정자 추가됨
@Service("googleDrive")어노테이션을 추가하여 Bean의 이름을 명시적으로 지정한 것은 좋은 변경입니다. 이를 통해 여러 ClubImageService 구현체 간의 명확한 구분이 가능해졌습니다.
138-138: 파일명 검증 로직 개선됨한글 포함 여부 확인에서 한글 또는 공백 포함 여부 확인으로 조건을 개선하였습니다. 이로써 공백이 포함된 파일명도 랜덤 문자열로 대체되어 파일명 관련 문제를 방지할 수 있습니다.
27-171:Details
❌ Incorrect review comment
Cloudflare 관련 구현체 누락 확인 필요
PR 목표에 따르면 이 PR은 Cloudflare 이미지 저장 기능을 구현하는 것입니다. 그러나 검토된 파일 중에는
CloudflareImageService또는 S3 구성 관련 파일이 보이지 않습니다. CloudflareImageService 구현과 S3 구성이 별도의 파일에 있는지 확인하세요.
🏁 Script executed:
#!/bin/bash # Cloudflare 관련 구현체 및 S3 구성 확인 echo "CloudflareImageService 클래스 및 S3 구성 확인:" rg -l "CloudflareImageService" --type java rg -l "S3Config" --type java # S3 관련 구성 확인 echo -e "\nS3/Cloudflare 관련 구성 확인:" fd -e yml -e yaml -e properties | xargs grep -l "s3\|cloudflare" 2>/dev/null # 새로 추가된 컨트롤러 확인 echo -e "\nCloudflare 컨트롤러 확인:" rg -l "CloudflareImageController" --type javaLength of output: 594
CloudflareImageService 및 S3Config 구현체 확인됨
검토 결과, PR에 아래 파일들이 모두 포함되어 있어 구현체가 누락되지 않았습니다.
- backend/src/main/java/moadong/media/service/CloudflareImageService.java
- backend/src/main/java/moadong/media/util/S3Config.java
- backend/src/main/java/moadong/media/controller/CloudflareImageController.java
추가로 확인된 바, YAML/Properties 설정 파일에서
s3또는cloudflare관련 설정이 별도로 필요하다면 해당 파일에 추가해 주세요. 그 외에는 별도 조치가 필요하지 않습니다.Likely an incorrect or invalid review comment.
backend/src/main/java/moadong/media/util/S3Config.java (1)
18-25:Details
✅ Verification successful
환경 설정 값의 안전한 관리 확인 필요
AWS 자격 증명과 엔드포인트 정보를 properties 파일에서 가져오고 있습니다. 이러한 민감한 정보가 소스 코드 관리 시스템에 포함되지 않고 적절히 관리되고 있는지 확인해주세요.
다음 스크립트를 실행하여 민감한 자격 증명이 소스 코드에 포함되어 있는지 확인해보세요:
🏁 Script executed:
#!/bin/bash # AWS 자격 증명이 소스 코드에 직접 포함되어 있는지 확인 echo "Checking for potential hardcoded AWS credentials in properties files:" rg -i "(accessKey|secretKey|aws.*credentials)" --type propertiesLength of output: 250
다음 스크립트를 실행하여
properties및 YAML 파일 내에 하드코딩된 AWS 자격 증명이 포함되어 있는지 재확인해주세요:
🏁 Script executed:
#!/bin/bash echo "properties 및 YAML 파일에서 하드코딩된 AWS 자격 증명 확인 중…" rg -i "(aws_access_key_id|aws_secret_access_key|accessKey|secretKey)" -g '*.properties' -g '*.yml' -g '*.yaml'Length of output: 211
하드코딩된 AWS 자격 증명 없음 확인
.properties및.yml/.yaml파일에서accessKey,secretKey,aws_access_key_id,aws_secret_access_key키워드를 검색한 결과, 하드코딩된 자격 증명은 전혀 발견되지 않았습니다.
환경 변수나 시크릿 매니저로 주입되고 있으며, VCS에는 민감 정보가 포함되어 있지 않음을 확인했습니다.추가 권장 사항:
application.properties/application.yml등 설정 파일을 커밋할 때는.gitignore설정을 반드시 검토해주세요.- AWS Secrets Manager, Parameter Store, HashiCorp Vault 등 시크릿 관리 도구를 사용해 자격 증명을 주입하는 방식을 유지하세요.
- 로컬 개발 전용 프로퍼티 파일(
application-local.properties등)을 별도 관리하고 VCS에서 제외하도록 구성하십시오.이상으로 본 이슈는 해결된 것으로 보입니다.
backend/src/main/java/moadong/media/controller/CloudflareImageController.java (4)
36-42: LGTM! 로고 이미지 업로드 기능 구현이 적절합니다.Cloudflare 저장소에 대한 로고 이미지 업로드 기능이 잘 구현되어 있으며, 설명에도 명확하게 "cloudflare 상에" 라고 표시되어 있어 좋습니다.
44-49: LGTM! 로고 이미지 삭제 기능 구현이 적절합니다.Cloudflare 저장소에서 로고 이미지를 삭제하는 API가 잘 구현되어 있습니다.
51-55: LGTM! 피드 이미지 업로드 기능 구현이 적절합니다.Cloudflare 저장소에 피드 이미지 업로드 기능이 잘 구현되어 있습니다.
57-62: LGTM! 피드 이미지 업데이트 기능 구현이 적절합니다.Cloudflare 저장소의 피드 이미지 설정을 업데이트하는 기능이 잘 구현되어 있습니다.
backend/src/main/java/moadong/media/service/CloudflareImageService.java (6)
40-52: 로고 업로드 로직 구현이 잘 되어 있습니다.로고 업로드 로직이 적절하게 구현되어 있습니다. 기존 로고가 있는 경우 먼저 삭제하고, 새 로고를 업로드한 후 클럽 엔티티를 업데이트하는 흐름이 잘 구성되어 있습니다.
54-63: 로고 삭제 로직 구현이 잘 되어 있습니다.로고 삭제 로직이 적절하게 구현되어 있습니다. 로고가 존재하는 경우에만 삭제하고, 엔티티 정보도 업데이트하는 흐름이 잘 구성되어 있습니다.
65-73: 피드 이미지 업로드 시 최대 개수 검증 구현이 적절합니다.피드 이미지 업로드 전에 최대 개수를 검증하는 로직이 잘 구현되어 있습니다. 최대 개수를 초과하는 경우 적절한 예외를 발생시키는 것도 좋습니다.
75-90: 피드 이미지 업데이트 로직 구현이 잘 되어 있습니다.피드 이미지 목록 업데이트 로직이 잘 구현되어 있습니다. 최대 개수 검증, 제거된 이미지 삭제, 엔티티 업데이트 등의 흐름이 잘 구성되어 있습니다.
92-96: 클럽 조회 Helper 메서드가 적절하게 구현되어 있습니다.클럽 ID를 받아 ObjectId로 변환하고 클럽을 조회하는 Helper 메서드가 잘 구현되어 있습니다. 클럽이 없는 경우 적절한 예외를 발생시키는 것도 좋습니다.
98-104: 삭제할 피드 이미지를 찾는 로직이 효율적입니다.새 피드 이미지 목록에 없는 기존 이미지를 찾아 삭제하는 로직이 효율적으로 구현되어 있습니다.
| private String uploadFile(String clubId, MultipartFile file, FileType fileType) { | ||
| if (file == null || file.isEmpty()) { | ||
| throw new RestApiException(ErrorCode.FILE_NOT_FOUND); | ||
| } | ||
|
|
||
| // 파일명 처리 | ||
| String fileName = file.getOriginalFilename(); | ||
| if (containsKoreanOrBlank(fileName)) { | ||
| fileName = RandomStringUtil.generateRandomString(10); | ||
| } | ||
|
|
||
| // S3에 저장할 key 경로 생성 | ||
| String key = clubId + "/" + fileType + "/" + fileName; | ||
|
|
||
| // S3 업로드 요청 | ||
| try { | ||
| PutObjectRequest putRequest = PutObjectRequest.builder() | ||
| .bucket(bucketName) | ||
| .key(key) | ||
| .contentType(file.getContentType()) | ||
| .acl(ObjectCannedACL.PUBLIC_READ) // 공개 URL 용도 | ||
| .build(); | ||
|
|
||
| s3Client.putObject(putRequest, RequestBody.fromInputStream( | ||
| file.getInputStream(), | ||
| file.getSize() | ||
| )); | ||
|
|
||
| } catch (IOException e) { | ||
| throw new RestApiException(ErrorCode.FILE_TRANSFER_ERROR); | ||
| } catch (Exception e) { | ||
| throw new RestApiException(ErrorCode.IMAGE_UPLOAD_FAILED); | ||
| } | ||
|
|
||
| // 공유 가능한 공개 URL 반환 | ||
| return viewEndpoint + "/" + key; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
파일 업로드 로직 고려사항
파일 업로드 로직은 잘 구현되어 있으나 몇 가지 고려해볼 사항이 있습니다:
- 파일 크기 제한 검사가 없습니다. 너무 큰 파일이 업로드 되는 것을 방지하기 위한 검사를 추가하는 것이 좋습니다.
- 파일 확장자 검증이 없습니다. 보안상 허용된 이미지 형식(예: jpg, png, gif 등)만 업로드 되도록 검증하는 것이 좋습니다.
- 랜덤 파일명에 원본 파일 확장자를 유지하는 것이 좋습니다. 그렇지 않으면 브라우저가 MIME 타입으로만 파일 형식을 인식해야 합니다.
private String uploadFile(String clubId, MultipartFile file, FileType fileType) {
if (file == null || file.isEmpty()) {
throw new RestApiException(ErrorCode.FILE_NOT_FOUND);
}
+ // 파일 크기 제한 검사 (예: 10MB)
+ long maxSize = 10 * 1024 * 1024; // 10MB
+ if (file.getSize() > maxSize) {
+ throw new RestApiException(ErrorCode.FILE_TOO_LARGE);
+ }
+
+ // 이미지 파일 형식 검증
+ String contentType = file.getContentType();
+ if (contentType == null || !contentType.startsWith("image/")) {
+ throw new RestApiException(ErrorCode.INVALID_FILE_TYPE);
+ }
// 파일명 처리
String fileName = file.getOriginalFilename();
+ String fileExtension = "";
+ if (fileName != null && fileName.contains(".")) {
+ fileExtension = fileName.substring(fileName.lastIndexOf("."));
+ }
+
if (containsKoreanOrBlank(fileName)) {
- fileName = RandomStringUtil.generateRandomString(10);
+ fileName = RandomStringUtil.generateRandomString(10) + fileExtension;
}
// S3에 저장할 key 경로 생성
String key = clubId + "/" + fileType + "/" + fileName;📝 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 String uploadFile(String clubId, MultipartFile file, FileType fileType) { | |
| if (file == null || file.isEmpty()) { | |
| throw new RestApiException(ErrorCode.FILE_NOT_FOUND); | |
| } | |
| // 파일명 처리 | |
| String fileName = file.getOriginalFilename(); | |
| if (containsKoreanOrBlank(fileName)) { | |
| fileName = RandomStringUtil.generateRandomString(10); | |
| } | |
| // S3에 저장할 key 경로 생성 | |
| String key = clubId + "/" + fileType + "/" + fileName; | |
| // S3 업로드 요청 | |
| try { | |
| PutObjectRequest putRequest = PutObjectRequest.builder() | |
| .bucket(bucketName) | |
| .key(key) | |
| .contentType(file.getContentType()) | |
| .acl(ObjectCannedACL.PUBLIC_READ) // 공개 URL 용도 | |
| .build(); | |
| s3Client.putObject(putRequest, RequestBody.fromInputStream( | |
| file.getInputStream(), | |
| file.getSize() | |
| )); | |
| } catch (IOException e) { | |
| throw new RestApiException(ErrorCode.FILE_TRANSFER_ERROR); | |
| } catch (Exception e) { | |
| throw new RestApiException(ErrorCode.IMAGE_UPLOAD_FAILED); | |
| } | |
| // 공유 가능한 공개 URL 반환 | |
| return viewEndpoint + "/" + key; | |
| } | |
| private String uploadFile(String clubId, MultipartFile file, FileType fileType) { | |
| if (file == null || file.isEmpty()) { | |
| throw new RestApiException(ErrorCode.FILE_NOT_FOUND); | |
| } | |
| // 파일 크기 제한 검사 (예: 10MB) | |
| long maxSize = 10 * 1024 * 1024; // 10MB | |
| if (file.getSize() > maxSize) { | |
| throw new RestApiException(ErrorCode.FILE_TOO_LARGE); | |
| } | |
| // 이미지 파일 형식 검증 | |
| String contentType = file.getContentType(); | |
| if (contentType == null || !contentType.startsWith("image/")) { | |
| throw new RestApiException(ErrorCode.INVALID_FILE_TYPE); | |
| } | |
| // 파일명 처리 | |
| String fileName = file.getOriginalFilename(); | |
| String fileExtension = ""; | |
| if (fileName != null && fileName.contains(".")) { | |
| fileExtension = fileName.substring(fileName.lastIndexOf(".")); | |
| } | |
| if (containsKoreanOrBlank(fileName)) { | |
| fileName = RandomStringUtil.generateRandomString(10) + fileExtension; | |
| } | |
| // S3에 저장할 key 경로 생성 | |
| String key = clubId + "/" + fileType + "/" + fileName; | |
| // S3 업로드 요청 | |
| try { | |
| PutObjectRequest putRequest = PutObjectRequest.builder() | |
| .bucket(bucketName) | |
| .key(key) | |
| .contentType(file.getContentType()) | |
| .acl(ObjectCannedACL.PUBLIC_READ) // 공개 URL 용도 | |
| .build(); | |
| s3Client.putObject(putRequest, RequestBody.fromInputStream( | |
| file.getInputStream(), | |
| file.getSize() | |
| )); | |
| } catch (IOException e) { | |
| throw new RestApiException(ErrorCode.FILE_TRANSFER_ERROR); | |
| } catch (Exception e) { | |
| throw new RestApiException(ErrorCode.IMAGE_UPLOAD_FAILED); | |
| } | |
| // 공유 가능한 공개 URL 반환 | |
| return viewEndpoint + "/" + key; | |
| } |
🤖 Prompt for AI Agents
In backend/src/main/java/moadong/media/service/CloudflareImageService.java
between lines 119 and 155, the uploadFile method lacks file size and extension
validation. Add a check to reject files exceeding a defined maximum size to
prevent overly large uploads. Implement validation to allow only specific image
file extensions like jpg, png, and gif for security. When generating a random
filename due to Korean characters or blanks, preserve the original file
extension by extracting it from the original filename and appending it to the
random string.
There was a problem hiding this comment.
코드레빗 답변처럼 파일 크기 제한 및 확장자 검증은 필요하지 않을까요?
There was a problem hiding this comment.
@Zepelown 네, 코드레빗의 제안처럼 파일 크기 제한 및 확장자 검증은 매우 필요합니다. 다음과 같은 이유로 구현하는 것이 좋습니다:
- 파일 크기 제한: 대용량 파일 업로드로 인한 서버 부하 방지와 스토리지 비용 관리를 위해 필요합니다.
- 확장자 검증: 악성 파일 업로드를 차단하여 보안 위험을 줄이고, 이미지 파일만 허용하도록 제한할 수 있습니다.
- 확장자 유지: 랜덤 파일명 생성 시 원본 확장자를 유지해야 브라우저가 파일 형식을 올바르게 인식하고 처리할 수 있습니다.
이러한 검증은 사용자 경험과 시스템 보안 모두에 중요한 영향을 미치므로 반드시 구현하는 것이 좋습니다.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Zepelown
left a comment
Zepelown
left a comment
There was a problem hiding this comment.
고생하셨습니다.
궁금한 점이 있어 커맨트 남겼는데 답장 부탁드릴게요
| private String uploadFile(String clubId, MultipartFile file, FileType fileType) { | ||
| if (file == null || file.isEmpty()) { | ||
| throw new RestApiException(ErrorCode.FILE_NOT_FOUND); | ||
| } | ||
|
|
||
| // 파일명 처리 | ||
| String fileName = file.getOriginalFilename(); | ||
| if (containsKoreanOrBlank(fileName)) { | ||
| fileName = RandomStringUtil.generateRandomString(10); | ||
| } | ||
|
|
||
| // S3에 저장할 key 경로 생성 | ||
| String key = clubId + "/" + fileType + "/" + fileName; | ||
|
|
||
| // S3 업로드 요청 | ||
| try { | ||
| PutObjectRequest putRequest = PutObjectRequest.builder() | ||
| .bucket(bucketName) | ||
| .key(key) | ||
| .contentType(file.getContentType()) | ||
| .acl(ObjectCannedACL.PUBLIC_READ) // 공개 URL 용도 | ||
| .build(); | ||
|
|
||
| s3Client.putObject(putRequest, RequestBody.fromInputStream( | ||
| file.getInputStream(), | ||
| file.getSize() | ||
| )); | ||
|
|
||
| } catch (IOException e) { | ||
| throw new RestApiException(ErrorCode.FILE_TRANSFER_ERROR); | ||
| } catch (Exception e) { | ||
| throw new RestApiException(ErrorCode.IMAGE_UPLOAD_FAILED); | ||
| } | ||
|
|
||
| // 공유 가능한 공개 URL 반환 | ||
| return viewEndpoint + "/" + key; | ||
| } |
There was a problem hiding this comment.
코드레빗 답변처럼 파일 크기 제한 및 확장자 검증은 필요하지 않을까요?
확장자의 경우 프론트측에서 확인을 한 번 넘어오는 것으로 알고있는데 백엔드에서 한 번 더 검증하는 것도 나쁘지 않을 것 같아 검증하는 방식으로 수정하겠습니다! |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
backend/src/main/java/moadong/media/service/CloudflareImageService.java (1)
123-143:⚠️ Potential issue파일 확장자 유효성 검사 후 랜덤 파일명 생성 시 확장자 유지 필요
파일 업로드 시 파일명에 한글이나 특수문자가 포함된 경우 랜덤 문자열로 변경하는 로직이 있으나, 원본 파일의 확장자가 유지되지 않습니다. 이는 파일 타입 식별과 브라우저의 올바른 처리를 위해 중요합니다.
private String uploadFile(String clubId, MultipartFile file, FileType fileType) { if (file == null || file.isEmpty()) { throw new RestApiException(ErrorCode.FILE_NOT_FOUND); } // 파일명 처리 String fileName = file.getOriginalFilename(); + String fileExtension = ""; + if (fileName != null && fileName.contains(".")) { + fileExtension = fileName.substring(fileName.lastIndexOf(".")); + } if (!isImageExtension(fileName)) { throw new RestApiException(ErrorCode.UNSUPPORTED_FILE_TYPE); } if (containsInvalidChars(fileName)) { - fileName = RandomStringUtil.generateRandomString(10); + fileName = RandomStringUtil.generateRandomString(10) + fileExtension; }
🧹 Nitpick comments (5)
backend/src/main/java/moadong/media/util/ClubImageUtil.java (2)
15-20: containsInvalidChars 메소드에 JavaDoc 추가 필요
containsInvalidChars메소드에 대한 문서화가 부족합니다. 메소드가 무엇을 검사하는지, 어떤 종류의 문자가 유효하지 않은지를 설명하는 JavaDoc 주석을 추가하면 더 명확할 것입니다.+ /** + * 파일명에 유효하지 않은 문자가 포함되어 있는지 확인합니다. + * 유효하지 않은 문자: + * - 퍼센트 인코딩 시퀀스 (%20, %3A 등) + * - 한글 문자 (한글 및 자모 범위) + * - 공백 문자 (스페이스, 탭 등) + * + * @param text 검사할 파일명 또는 텍스트 + * @return 유효하지 않은 문자가 포함되어 있으면 true, 그렇지 않으면 false + */ public static boolean containsInvalidChars(String text) { text = Normalizer.normalize(text, Normalizer.Form.NFC); return Pattern.matches(".*(%[0-9A-Fa-f]{2}|[ㄱ-ㅎㅏ-ㅣ가-힣]|\\s).*", text); }
22-28: isImageExtension 메소드 개선 사항
isImageExtension메소드가 추가되어 이미지 확장자를 효과적으로 검증하고 있습니다. 다만 JavaDoc 주석을 추가하여 메소드의 용도와 파라미터, 반환값에 대한 설명을 추가하면 더 좋을 것입니다.+ /** + * 주어진 파일명의 확장자가 허용된 이미지 확장자 중 하나인지 확인합니다. + * + * @param originalFilename 확인할 파일명 + * @return 확장자가 허용되면 true, 그렇지 않으면 false + */ public static boolean isImageExtension(String originalFilename) { if (originalFilename == null || !originalFilename.contains(".")) { return false; } String extension = originalFilename.substring(originalFilename.lastIndexOf(".") + 1).toLowerCase(); return ALLOWED_IMAGE_EXTENSIONS.contains(extension); }backend/src/main/java/moadong/media/service/CloudflareImageService.java (3)
149-166: S3 업로드 로직 최적화 제안예외 처리는 잘 되어 있으나, I/O 예외와 일반 예외를 구분하여 처리하는 것보다 좀 더 구체적인 AWS SDK 관련 예외를 처리하는 것이 좋을 것입니다. 또한
S3Exception과 같은 특정 예외를 캐치하면 더 명확한 오류 메시지를 제공할 수 있습니다.try { PutObjectRequest putRequest = PutObjectRequest.builder() .bucket(bucketName) .key(key) .contentType(file.getContentType()) .acl(ObjectCannedACL.PUBLIC_READ) // 공개 URL 용도 .build(); s3Client.putObject(putRequest, RequestBody.fromInputStream( file.getInputStream(), file.getSize() )); } catch (IOException e) { + // 로깅 추가 + // logger.error("파일 스트림 처리 중 오류 발생", e); throw new RestApiException(ErrorCode.FILE_TRANSFER_ERROR); - } catch (Exception e) { + } catch (software.amazon.awssdk.services.s3.model.S3Exception e) { + // 로깅 추가 + // logger.error("S3 업로드 중 오류 발생: {}", e.getMessage(), e); throw new RestApiException(ErrorCode.IMAGE_UPLOAD_FAILED); + } catch (Exception e) { + // 로깅 추가 + // logger.error("파일 업로드 중 예상치 못한 오류 발생", e); + throw new RestApiException(ErrorCode.INTERNAL_SERVER_ERROR); }
96-100: getClub 메소드의 효율적인 사용
getClub메소드가 클럽 조회를 위한 공통 로직을 잘 캡슐화하고 있습니다. 하지만uploadFeed메소드에서는 동일한 클럽 객체를 두 번 조회하게 되어 비효율적입니다.uploadFeed메소드를 아래와 같이 개선할 수 있습니다:@Override public String uploadFeed(String clubId, MultipartFile file) { - int feedImagesCount = getClub(clubId).getClubRecruitmentInformation().getFeedImages().size(); + Club club = getClub(clubId); + int feedImagesCount = club.getClubRecruitmentInformation().getFeedImages().size(); if (feedImagesCount + 1 > MAX_FEED_COUNT) { throw new RestApiException(ErrorCode.TOO_MANY_FILES); } return uploadFile(clubId, file, FileType.FEED); }
102-108: deleteFeedImages 메소드에서 더 효율적인 컬렉션 처리현재 구현은 모든 피드 이미지를 순회하며 새 목록에 없는 이미지를 삭제하고 있습니다. 컬렉션 스트림 API를 사용하여 더 간결하고 효율적으로 구현할 수 있습니다.
private void deleteFeedImages(Club club, List<String> feedImages, List<String> newFeedImages) { - for (String feedsImage : feedImages) { - if (!newFeedImages.contains(feedsImage)) { - deleteFile(club, feedsImage); - } - } + feedImages.stream() + .filter(image -> !newFeedImages.contains(image)) + .forEach(image -> deleteFile(club, image)); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
backend/build.gradle(1 hunks)backend/src/main/java/moadong/global/exception/ErrorCode.java(1 hunks)backend/src/main/java/moadong/media/service/CloudflareImageService.java(1 hunks)backend/src/main/java/moadong/media/service/GcsClubImageService.java(2 hunks)backend/src/main/java/moadong/media/service/GoogleDriveClubImageService.java(3 hunks)backend/src/main/java/moadong/media/util/ClubImageUtil.java(1 hunks)backend/src/test/java/moadong/media/service/GoogleDriveClubImageServiceLogoTest.java(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- backend/src/main/java/moadong/global/exception/ErrorCode.java
🚧 Files skipped from review as they are similar to previous changes (3)
- backend/src/main/java/moadong/media/service/GcsClubImageService.java
- backend/build.gradle
- backend/src/main/java/moadong/media/service/GoogleDriveClubImageService.java
🧰 Additional context used
🧬 Code Graph Analysis (1)
backend/src/test/java/moadong/media/service/GoogleDriveClubImageServiceLogoTest.java (1)
backend/src/main/java/moadong/media/util/ClubImageUtil.java (1)
ClubImageUtil(13-66)
🔇 Additional comments (3)
backend/src/test/java/moadong/media/service/GoogleDriveClubImageServiceLogoTest.java (2)
133-151: 테스트 케이스 구현이 적절합니다.다양한 파일명 패턴에 대해
containsInvalidChars메소드의 동작을 검증하는 테스트가 잘 구현되어 있습니다. 정상 케이스, 한글 포함, 퍼센트 인코딩, 공백 포함, 혼합 케이스 등 다양한 시나리오를 포함하고 있어 유틸리티 메소드의 정확성을 확인하는데 충분합니다.
4-7:Details
✅ Verification successful
불필요한 import 정리 확인
assertFalse와assertTrue를 추가한 것은 적절하나, 이 파일에서UnitTest애노테이션이 추가된 것도 확인되었습니다. 이 변경이 의도적인지 확인해주세요.
🏁 Script executed:
#!/bin/bash # UnitTest 애노테이션의 사용 위치와 용도 확인 rg -A 3 "@UnitTest" --type javaLength of output: 1512
🏁 Script executed:
#!/bin/bash # static import 사용 여부 확인 rg -n "assertNull" backend/src/test/java/moadong/media/service/GoogleDriveClubImageServiceLogoTest.java rg -n "assertThrows" backend/src/test/java/moadong/media/service/GoogleDriveClubImageServiceLogoTest.javaLength of output: 714
@UnitTest 애노테이션 및 static import 일관성 확인 완료
assertFalse,assertTrue,assertNull,assertThrows모두 해당 테스트 메소드에서 실제로 사용되고 있어 불필요한 import가 없습니다.GoogleDriveClubImageServiceLogoTest클래스에 추가된@UnitTest애노테이션은 UserRegisterTest, UserLoginTest, ClubProfileServiceTest 등 다른 유닛 테스트 클래스에도 일관되게 적용되어 있어 의도된 변경입니다.위 변경 사항 모두 적절하므로 별도 수정 없이 머지 가능합니다.
backend/src/main/java/moadong/media/service/CloudflareImageService.java (1)
27-42: 서비스 설정 및 의존성 주입 구현이 잘 되어 있습니다."cloudflare"라는 명시적인 서비스 이름을 지정하고, 필요한 의존성(ClubRepository, S3Client)과 설정값(MAX_FEED_COUNT, bucketName 등)을 적절히 주입받고 있습니다. 이렇게 명시적인 서비스 이름을 지정함으로써 다른
ClubImageService구현체와 구분하여 사용할 수 있습니다.
lepitaaar
left a comment
lepitaaar
left a comment
There was a problem hiding this comment.
좋네용, 혹시 webp로 변환해 저장하실 생각없으신가요?
jpg로도 충분히 원하는정도의 용량을 만들 수 있고, 피드 이미지와 로고 모두 최대갯수가 정해져 저장용량에 문제가 없다고 생각했습니다. webp의 경우 이전 버전의 브라우저 호환성에서 문제가 있을수도있다는 얘기가 있어 저희 프로젝트에는 jpg가 조금 더 적절하다고 생각해 jpg로 압축하는걸 선택했습니다. 추후 논의나 방침을 통해 수정할 가능성은 열어두고있습니다~ |
| return ALLOWED_IMAGE_EXTENSIONS.contains(extension); | ||
| } | ||
|
|
||
| public static MultipartFile resizeImage(MultipartFile file, long maxSizeBytes) throws IOException { |
4 participants
#️⃣연관된 이슈
📝작업 내용
Summary by CodeRabbit
신규 기능
버그 수정
리팩터
기타