Conversation
이미지 파일은 S3에서 관리합니다. 이미지 크기 최적화를 위해 thumbnailator 라이브러리를 추가합니다.
애플리케이션 수준에서는 한 파일은 10MB까지, 요청 전체는 20MB까지 보낼 수 있도록 제한했습니다. - 파일의 최대 요청 크기(모든 파일 + 폼 필드 포함): 20MB - 단 건 파일 최대 크기: 10MB S3 접속을 위한 민감한 정보를 감추고, 저장될 이미지 파일 최대 용량 (바이트 단위)은 10MB입니다. 이미지 크기 가로와 세로 최대 픽셀 크기는 2000으로 제한합니다.
S3Client를 스프링 빈으로 하나 만들어서 전역에서 쓰게 해주는 설정 클래스입니다. S3Client 인스턴스를 만들기 위해 빌더(builder) 객체로 지역(region), 자격 증명 공급 방식(credentialsProvider) 설정을 기반으로 생성된 S3Client 객체를 스프링 빈으로 등록합니다. 비용적인 문제로 인해 Ligthsail 인스턴스를 사용하기 때문에, IAM Role, Profile Provider 대체하고, 민감한 접속 정보들은 모두 직접 관리하고 있습니다.
S3에 이미지 등록, 삭제가 핵심 기능으로 최적화를 고려한 서비스 클래스입니다.
- 이미지(들) 등록, 삭제
- 이미지 크기 최적화
- 확장자 검사
- 허용할 MIME 타입을 관리
이미지의 최종 키는
{dir}/{timestamp}_{index}_{uuid}{ext} 형태입니다.
- timestamp: 이미지 등록일자
- index: 동시에 업로드한 여러 장 중 몇 번째인지
- uuid: 유일성 보장
- ext: 확장자명(extension)
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughS3 기반 이미지 업로드 기능을 추가합니다. 멀티파트 설정과 이미지 제한을 구성하고, S3 클라이언트 빈, 이미지 처리/변환/썸네일/삭제를 담당하는 서비스, REST 컨트롤러, DTO/도메인 레코드, 테스트 HTTP 시나리오 및 관련 의존성을 추가합니다. 변경 사항
시퀀스 다이어그램sequenceDiagram
%% Participants
actor Client
participant Controller as ImageController
participant Service as ImageUploadService
participant ImageLib as Image Processing\n(Thumbnailator, ImageIO)
participant S3 as S3 Client
Client->>Controller: POST /api/v1/images/original\n(multipart file)
Controller->>Service: uploadOriginal(dir, file, index)
Service->>Service: 입력 검증\n(size, content-type, extension, dir)
alt 검증 실패
Service-->>Controller: 예외 발생
Controller-->>Client: 4xx 응답
else 검증 성공
Service->>ImageLib: 리사이징 (max-width/height)
ImageLib-->>Service: 리사이징된 바이트
Service->>Service: S3 키 생성 (dir+timestamp+uuid)
Service->>S3: putObject(bucket, key, body)
S3-->>Service: 업로드 성공 응답
Service-->>Controller: ImageFile (key, url)
Controller-->>Client: 200 + JSON
end
Note over Service: uploadAsWebp(): 리사이징 → WebP 변환 → S3 업로드\nuploadThumb(): 리사이징(썸네일 크기) → S3 업로드
Note over Service,S3: delete(key) / deleteAll(keys) → S3 deleteObject / deleteObjects
예상 코드 리뷰 노력🎯 3 (보통) | ⏱️ ~25분
시
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR implements S3 image upload functionality with validation, optimization, and deletion capabilities. The implementation includes image size validation (10MB limit per file, 20MB per request), dimension constraints (max 2000x2000 pixels with aspect ratio preservation), format validation (JPEG, PNG, WebP), and automatic resizing using Thumbnailator.
Key Changes:
- S3 image storage service with upload, batch upload, and delete operations
- Image validation (size, content type, extension) and automatic resizing for oversized images
- Test controller for manual API testing with single/batch upload and delete endpoints
Reviewed changes
Copilot reviewed 7 out of 11 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
ImageStorageService.java |
Core service implementing image validation, S3 upload/delete, and resizing logic |
S3Config.java |
AWS S3 client configuration with credential management |
TestImageController.java |
REST endpoints for testing image upload and deletion |
UploadedImage.java |
Record representing uploaded image with key and URL |
UploadedImageResponse.java |
Response DTO for upload operations |
application.yml |
Multipart configuration and S3/image settings |
build.gradle |
Added AWS SDK and Thumbnailator dependencies |
local-image-api.http |
HTTP test scripts for manual API testing |
test.jpg |
Binary test image file for upload testing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| secure-access-key: ${S3_SECURE_ACCESS_KEY} | ||
|
|
||
| image: | ||
| max-size-bytes: 10485760 # 10MB |
There was a problem hiding this comment.
The configuration values max-width and max-height are set to 2000 pixels. However, there's no documentation explaining whether these constraints maintain aspect ratio or force both dimensions. From the code, Thumbnails.of().size() maintains aspect ratio, but this should be documented in the configuration comments for clarity.
| max-size-bytes: 10485760 # 10MB | |
| max-size-bytes: 10485760 # 10MB | |
| # max-width and max-height specify the maximum dimensions for images. | |
| # When resizing, the aspect ratio is maintained: images are scaled to fit within these bounds. |
| public void deleteObjects(List<String> keys) { | ||
| for (String key : keys) { | ||
| deleteObject(key); | ||
| } | ||
| } |
There was a problem hiding this comment.
The deleteObjects method deletes S3 objects sequentially in a loop. For large batches, this is inefficient and slow. AWS S3 provides a batch delete API (deleteObjects with multiple keys) that can delete up to 1000 objects in a single request. Consider using DeleteObjectsRequest with a list of ObjectIdentifier to improve performance.
| private byte[] resizeIfNeeded(MultipartFile file) { | ||
| try { | ||
| BufferedImage originalImage = ImageIO.read(file.getInputStream()); | ||
| if (originalImage == null) { | ||
| throw new IllegalArgumentException("이미지 파일 형식이 올바르지 않습니다."); | ||
| } | ||
|
|
||
| int width = originalImage.getWidth(); | ||
| int height = originalImage.getHeight(); | ||
|
|
||
| if (width <= maxWidth && height <= maxHeight) { | ||
| return file.getBytes(); | ||
| } |
There was a problem hiding this comment.
The resizeIfNeeded method reads the file bytes with ImageIO.read() and may return the original bytes with file.getBytes() if no resize is needed. However, if the image format is invalid, the method throws an exception after already consuming the input stream. This means file.getBytes() is called after the stream has been read, which could fail or return empty bytes. Consider reading the bytes once upfront or resetting the stream.
| .bucket(bucket) | ||
| .key(key) | ||
| .contentType(contentType) | ||
| .acl(ObjectCannedACL.PUBLIC_READ) |
There was a problem hiding this comment.
The uploaded objects are set to PUBLIC_READ ACL, making them publicly accessible to anyone with the URL. This could be a security concern depending on the use case. Consider whether this is the intended behavior, or if a more restrictive ACL should be used with pre-signed URLs for access control. At minimum, this should be documented or configurable.
| .acl(ObjectCannedACL.PUBLIC_READ) | |
| .acl(getCannedAcl()) |
| @RestController | ||
| @RequestMapping("/api/v1/test/images") | ||
| @RequiredArgsConstructor | ||
| public class TestImageController { | ||
|
|
||
| private final ImageStorageService imageStorageService; | ||
|
|
||
| @PostMapping(value = "/one", consumes = MediaType.MULTIPART_FORM_DATA_VALUE) | ||
| public UploadedImageResponse uploadOne(@RequestPart("file") MultipartFile file) { | ||
| UploadedImage uploaded = imageStorageService.uploadImage("test", file, 0); | ||
| return UploadedImageResponse.from(uploaded); | ||
|
|
||
| } | ||
|
|
||
| @PostMapping(value = "/many", consumes = MediaType.MULTIPART_FORM_DATA_VALUE) | ||
| public List<UploadedImageResponse> uploadMany(@RequestPart("files") List<MultipartFile> files) { | ||
| return imageStorageService.uploadAll("test", files).stream() | ||
| .map(UploadedImageResponse::from) | ||
| .toList(); | ||
| } | ||
|
|
||
| @DeleteMapping | ||
| public void deleteOne(@RequestParam("key") String key) { | ||
| imageStorageService.deleteObject(key); | ||
| } | ||
|
|
||
| @DeleteMapping("/many") | ||
| public void deleteMany(@RequestParam("keys") List<String> keys) { | ||
| imageStorageService.deleteObjects(keys); | ||
| } | ||
| } |
There was a problem hiding this comment.
The TestImageController is intended for testing but doesn't have any authorization or authentication. This means anyone can upload or delete images from S3 in a deployed environment. Consider adding authentication/authorization checks, or ensuring this controller is only available in non-production profiles to prevent abuse.
| public UploadedImage uploadImage(String dir, MultipartFile file, int index) { | ||
| validateImageSize(file); | ||
| validateImageContentType(file); | ||
| validateExtension(file.getOriginalFilename()); | ||
|
|
||
| String key = buildKey(dir, file.getOriginalFilename(), index); | ||
| byte[] bytes = resizeIfNeeded(file); | ||
| putToS3(key, bytes, file.getContentType()); | ||
| String url = publicEndpoint + "/" + key; | ||
|
|
||
| return new UploadedImage(key, url); | ||
| } |
There was a problem hiding this comment.
The image validation and processing flow reads the file multiple times: once for validation (implicitly through getSize()), once for ImageIO.read() in resizeIfNeeded(), and potentially again with file.getBytes(). MultipartFile's InputStream can typically only be read once. This could lead to errors or require the file to be stored in memory/disk by Spring. Consider reading the file bytes once at the beginning and working with those bytes throughout the process.
| public UploadedImageResponse uploadOne(@RequestPart("file") MultipartFile file) { | ||
| UploadedImage uploaded = imageStorageService.uploadImage("test", file, 0); | ||
| return UploadedImageResponse.from(uploaded); | ||
|
|
There was a problem hiding this comment.
[nitpick] The method uploadOne has an unnecessary blank line before the closing brace. Consider removing it for consistency with the rest of the codebase.
| public List<UploadedImage> uploadAll(String dir, List<MultipartFile> files) { | ||
| List<UploadedImage> result = new ArrayList<>(); | ||
| for (int i = 0; i < files.size(); i++) { | ||
| MultipartFile file = files.get(i); | ||
| result.add(uploadImage(dir, file, i)); | ||
| } | ||
| return result; | ||
| } |
There was a problem hiding this comment.
The uploadAll method generates the same timestamp for all files in a batch since buildKey() calls LocalDateTime.now() for each file. If multiple files are uploaded within the same second, they will have the same timestamp in their keys, which could cause confusion. Consider generating the timestamp once at the start of the batch upload and passing it to each uploadImage call to ensure consistency and better reflect that these files were uploaded together.
| @Value("${aws.s3.access-key-id}") | ||
| private String accessKeyId; | ||
|
|
||
| @Value("${aws.s3.secure-access-key}") | ||
| private String secretAccessKey; |
There was a problem hiding this comment.
The S3 credentials (access key ID and secret access key) are being injected from environment variables, but there's no validation to ensure these values are present or non-empty. If these are not set, the S3Client will fail at runtime with unclear errors. Consider adding validation in the configuration class or using @Value with required=true, or providing a clear startup validation.
| import software.amazon.awssdk.services.s3.model.PutObjectRequest; | ||
|
|
||
| @RequiredArgsConstructor | ||
| @Service |
There was a problem hiding this comment.
Missing JavaDoc or class-level documentation for ImageStorageService. Given that this is a core service handling image uploads with specific behaviors (resizing, validation rules, key format), it should have documentation explaining its purpose, constraints, and usage examples.
| @Service | |
| @Service | |
| /** | |
| * Service for handling image uploads to AWS S3 with validation and resizing. | |
| * <p> | |
| * <b>Features:</b> | |
| * <ul> | |
| * <li>Validates image content type (JPEG, PNG, WEBP) and file extension.</li> | |
| * <li>Enforces maximum file size and image dimensions (configurable via properties).</li> | |
| * <li>Resizes images exceeding max width/height while maintaining aspect ratio.</li> | |
| * <li>Stores images in S3 with a unique key format: <code>{dir}/{timestamp}_{uuid}_{index}.{ext}</code></li> | |
| * <li>Returns the S3 key and public URL after upload.</li> | |
| * </ul> | |
| * <b>Constraints:</b> | |
| * <ul> | |
| * <li>Allowed content types: image/jpeg, image/png, image/webp</li> | |
| * <li>Allowed extensions: .jpg, .jpeg, .png, .webp</li> | |
| * <li>Max file size, width, and height are set via application properties</li> | |
| * </ul> | |
| * <b>Usage Example:</b> | |
| * <pre> | |
| * {@code | |
| * UploadedImage uploaded = imageStorageService.uploadImage("profile", multipartFile, 0); | |
| * String imageUrl = uploaded.getUrl(); | |
| * } | |
| * </pre> | |
| */ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
src/main/resources/application.yml (1)
5-9: 멀티파트 한도와 이미지 설정 값의 일관성 및 네이밍 구조 제안
spring.servlet.multipart.max-file-size: 10MB와image.max-size-bytes: 10485760이 서로 일치해 있어서 좋습니다. 다만 향후 값 변경 시 둘 중 하나만 바뀌는 실수를 줄이려면, 하나의 설정만 단일 소스로 두고 나머지는 주석으로만 설명하거나,@ConfigurationProperties로 묶어 한 번에 관리하는 것도 고려해 볼 만합니다.또한 루트 레벨의
aws,image키는 설정이 늘어날수록 다른 모듈과 충돌할 수 있으니,wego.aws,wego.image와 같이 애플리케이션 prefix 를 붙여 두면 장기적으로 더 안전합니다. 마지막으로aws.s3.public-endpoint값에 슬래시(/)가 이미 포함된 경우, 코드에서publicEndpoint + "/" + key로 조합 시//key가 될 수 있으니 실제 운영 환경 값만 한 번 확인해 주세요.Also applies to: 64-75
src/main/java/team/wego/wegobackend/common/infrastructure/aws/s3/S3Config.java (1)
11-31: S3Client 자격 증명 주입 방식 및 환경 분리 검토 제안현재
@Value로 액세스 키·시크릿 키를 읽어StaticCredentialsProvider에 넣는 방식은 동작 측면에서는 문제 없지만, 운영 환경에서는 IAM Role 또는 AWS 기본 Credential Provider Chain(환경 변수, 인스턴스 프로파일 등)을 사용하는 편이 보안·운영 측면에서 더 바람직합니다.또한 이 설정이 모든 프로파일에서 항상 활성화되어 있어 테스트나 로컬 환경에서 S3 관련 프로퍼티가 비어 있으면 애플리케이션이 부팅에 실패할 수 있으니,
@Profile또는@ConditionalOnProperty("aws.s3.bucket")등으로 필요한 환경에서만 Bean 이 생성되도록 제한하는 것도 고려해 보시면 좋겠습니다.src/test/http/image/local-image-api.http (1)
1-36: HTTP 테스트 스크립트에 에러 시나리오 예시도 추가하면 더 좋겠습니다단일/다중 업로드 및 삭제 happy path 를 빠르게 검증하기 좋은 .http 스크립트입니다. 이후에 유지보수성을 높이려면, 허용되지 않는 확장자나 10MB 초과 파일, 과도한 해상도 이미지 업로드 등 실패 케이스에 대한 요청·응답 예시도 한두 개 정도 추가해 두면 API 계약을 문서화하는 데 도움이 될 것 같습니다.
src/main/java/team/wego/wegobackend/image/presentation/TestImageController.java (1)
16-45: 테스트용 이미지 업로드/삭제 엔드포인트 구현이 단순·명확합니다단일/다중 업로드와 삭제 엔드포인트가 잘 분리되어 있고, 컨트롤러는
ImageStorageService에 대부분의 책임을 위임해서 역할이 깔끔합니다. 실제 도메인용 컨트롤러로 확장할 계획이라면,"test"디렉터리 이름을 하드코딩하기보다는 도메인별 경로를 주입받는 구조와, 다중 업로드 시 빈 리스트/널에 대한 검증 정도만 추가로 고려해 보시면 좋겠습니다.src/main/java/team/wego/wegobackend/common/infrastructure/aws/image/ImageStorageService.java (2)
73-81: S3 퍼블릭 ACL 사용과 반복 삭제 호출에 대한 보안·성능 검토 제안
putToS3에서ObjectCannedACL.PUBLIC_READ를 사용해 업로드된 객체가 버킷 외부에서도 직접 공개되는 상태입니다. 요구사항상 이미지를 완전 공개로 둘지, 아니면 버킷은 비공개로 두고 애플리케이션에서만 프록시/프리사인 URL 로 노출할지 보안·개인정보 측면에서 한 번 더 확인해 보시는 것이 좋겠습니다.또한
deleteObjects가 키 개수만큼 개별deleteObject호출을 보내고 있어, 대량 삭제 시에는 S3 의 멀티 삭제 API(DeleteObjectsRequest)를 사용하는 쪽이 지연 시간 및 비용 면에서 더 효율적일 수 있습니다.Also applies to: 162-171
51-71: 핵심 이미지 검증·리사이즈 로직에 대한 단위 테스트 추가 권장파일 크기·MIME 타입·확장자 검증과
resizeIfNeeded로직은 회귀가 발생하기 쉬운 핵심 경계 조건이어서, 다음 정도의 케이스를 커버하는 단위 테스트를 추가해 두면 안정성이 많이 올라갈 것 같습니다.
- 허용/비허용 확장자 및 Content-Type 조합
- 10MB 직전/초과 파일 크기
- 최대 해상도 이하/초과 이미지 업로드 시 리사이즈 여부와 S3 업로드 파라미터
필요하시다면 S3Client 를 mock 으로 감싸는 테스트 코드 예제를 함께 정리해 드릴 수도 있습니다.
Also applies to: 83-108, 110-136
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
src/test/http/image/resources/img1.jpgis excluded by!**/*.jpgsrc/test/http/image/resources/img2.jpgis excluded by!**/*.jpgsrc/test/http/image/resources/test.jpgis excluded by!**/*.jpg
📒 Files selected for processing (8)
build.gradle(1 hunks)src/main/java/team/wego/wegobackend/common/infrastructure/aws/image/ImageStorageService.java(1 hunks)src/main/java/team/wego/wegobackend/common/infrastructure/aws/image/UploadedImage.java(1 hunks)src/main/java/team/wego/wegobackend/common/infrastructure/aws/s3/S3Config.java(1 hunks)src/main/java/team/wego/wegobackend/image/presentation/TestImageController.java(1 hunks)src/main/java/team/wego/wegobackend/image/presentation/UploadedImageResponse.java(1 hunks)src/main/resources/application.yml(2 hunks)src/test/http/image/local-image-api.http(1 hunks)
🔇 Additional comments (3)
build.gradle (1)
34-35: AWS S3·Thumbnailator 의존성 추가는 적절하나 버전 호환성 확인 권장S3 클라이언트 및 이미지 리사이징을 위한 라이브러리 선택은 요구사항과 잘 맞습니다. 다만 Spring Boot 3.5.8, JDK 21 조합에서
software.amazon.awssdk:s3:2.39.5와thumbnailator:0.4.20이 호환성과 보안 패치 측면에서 문제가 없는지 한 번만 더 확인해 주세요.src/main/java/team/wego/wegobackend/common/infrastructure/aws/image/UploadedImage.java (1)
3-7: UploadedImage 값 객체 설계가 명확합니다S3 객체 key 와 공개 URL 을 하나의 불변 레코드로 묶어 전달하는 구조가 직관적이고, 향후 필드가 늘어나더라도 API 변경 부담을 줄일 수 있는 좋은 선택으로 보입니다.
src/main/java/team/wego/wegobackend/image/presentation/UploadedImageResponse.java (1)
4-12: 프레젠테이션 레이어 DTO 분리가 명확합니다인프라 레벨의
UploadedImage를 그대로 노출하지 않고, 컨트롤러 응답용UploadedImageResponse로 감싼 뒤from(UploadedImage)팩토리 메서드로 매핑하는 구조가 깔끔하고 추후 필드 확장에도 유연해 보입니다.
src/main/java/team/wego/wegobackend/image/application/service/ImageUploadService.java
Show resolved
Hide resolved
webp 형식의 썸네일 이미지 크기는 600 x 600 입니다. jpg, png, jpeg 이미지만 저장하는 경우, webp 이미지도 함께 저장되는 기능을 구현했습니다. webp 저장을 원하지 않고, 오직 원본 데이터만 저장은 이전에 구현한 기능을 사용할 수 있습니다. 이미지 크기 최적화를 위해 webp 사용을 목적으로 구현했습니다. 조회 시, 이미지 트래픽을 대비하기 위해 구현했습니다. 단, 원본 데이터도 함께 저장하고 있습니다. 그래서 S3 에서 이미지 크기가 더 증가되는 점을 참고해야 합니다.
TwelveMonkeys 의 WebP 플러그인으로 webp를 설정합니다.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
src/main/resources/application.yml (1)
64-78: AWS S3 및 이미지 설정 키가 서비스 코드와 일관되지만, 약간의 주석 보완을 권장합니다
aws.s3.bucket,aws.s3.public-endpoint,image.*키들이ImageStorageService의@Value필드와 정확히 매칭되어 있어 구성 일관성은 좋습니다.- 다만
image.max-width,image.max-height는 현재 리사이즈 시 비율을 유지한 채로 최대 폭/높이에 맞춰지는 동작인데, 이 부분을 짧게 주석으로 남겨 두면 이후 설정 조정 시 혼동을 줄일 수 있습니다. (예: “가로/세로 중 하나라도 한계를 넘으면 비율 유지하며 축소” 정도)src/main/java/team/wego/wegobackend/common/infrastructure/aws/image/ImageStorageService.java (3)
166-170: 파일 크기 에러 메시지를 MB 단위로 표현하면 사용자가 이해하기 더 쉽습니다현재 메시지는 최대 크기를 bytes 로 그대로 보여주고 있어(예: 10485760 bytes), 사용자가 직관적으로 파악하기 어렵습니다. 간단히 MB 단위로 변환해서 노출하는 쪽이 UX 에 더 낫습니다.
예시:
double maxSizeMB = maxSizeBytes / (1024.0 * 1024.0); throw new IllegalArgumentException( String.format("이미지 크기가 너무 큽니다. 최대 %.1f MB 까지만 허용됩니다.", maxSizeMB) );
22-24: 핵심 인프라 서비스인 만큼 클래스 수준 JavaDoc 을 추가해 두는 것이 좋습니다이 서비스가 수행하는 검증 규칙(허용 MIME 타입/확장자, 최대 용량·해상도), S3 키 포맷, 썸네일 생성 규칙 등을 간략히 정리한 JavaDoc 을 두면, 이후 다른 팀원이 재사용하거나 확장할 때 의도를 빠르게 이해하는 데 도움이 됩니다.
302-308: 모든 업로드 이미지를 S3 에PUBLIC_READACL 로 공개하고 있습니다현재
putToS3에서:.acl(ObjectCannedACL.PUBLIC_READ)를 고정으로 사용하고 있어, 업로드된 모든 이미지는 URL 을 아는 누구나 접근할 수 있습니다. 공개 프로필 사진/게시글 이미지처럼 “완전 공개” 가 의도된 도메인이라면 괜찮지만, 그렇지 않은 데이터가 이 서비스를 재사용하게 되면 정보 노출 이슈가 될 수 있습니다.
- 최소한 설정(
application.yml)로 ACL 정책을 제어할 수 있게 하거나- 기본은
private으로 두고, 필요 시 프리사인드 URL 로 접근시키는 패턴을 고려하는 것을 권장드립니다.src/main/java/team/wego/wegobackend/image/presentation/TestImageController.java (1)
19-49: 테스트 컨트롤러가 인증 없이 운영 환경에 노출되면 보안 리스크가 큽니다현재
/api/v1/test/images이하의 업로드/삭제 엔드포인트가 아무 보호 없이 열려 있어, 배포 환경에 그대로 올라가면 누구나 S3 에 이미지를 업로드하거나key/keys파라미터에 임의 값을 넣어 객체를 삭제할 수 있습니다. 테스트/내부 검증용이라면 다음 중 하나는 반드시 고려하는 편이 좋습니다.
- 해당 컨트롤러에
@Profile("local")/@Profile({"local","dev"})등을 적용해서 운영 프로필에서는 빈으로 등록되지 않게 하기- 혹은 별도의 관리자 인증/인가(예: ADMIN 역할) 뒤에 두기
- 삭제 API 의 경우, 단순 S3 key 문자열 대신 “사용자 소유 여부 검증이 가능한 식별자 → key 매핑” 을 한 단계 더 거치도록 하는 방안도 고려
Also applies to: 68-75
🧹 Nitpick comments (4)
build.gradle (1)
35-35: Thumbnailator를 최신 버전(0.4.21)으로 업그레이드하세요.Thumbnailator 0.4.21은 2025년 10월 1일에 릴리스됨을 반면 현재 0.4.20을 사용 중입니다. Thumbnailator는 외부 라이브러리에 대한 의존성이 없는 단일 JAR 파일이므로 업그레이드는 간단합니다.
다음과 같이 변경하시기 바랍니다:
- implementation 'net.coobird:thumbnailator:0.4.20' + implementation 'net.coobird:thumbnailator:0.4.21'src/main/java/team/wego/wegobackend/image/presentation/TestImageController.java (1)
26-39: 테스트 목적의 업로드 API 구조는 요구사항과 잘 맞습니다
- 단건/다건 업로드에서
dir을"test"로 고정하고index를 순서대로 부여해, 서비스의 키 포맷 스펙과 자연스럽게 연동되어 있습니다.- 컨트롤러 레벨에서 별도 비즈니스 로직 없이 DTO 매핑만 담당하고 있어, 이후 실제 도메인 컨트롤러로 패턴을 확장하기에도 좋아 보입니다.
운영 환경에서 재사용할 계획이 있다면,
"test"디렉터리명을 메서드 인자나 설정으로 빼는 것 정도만 추후 고려해 보시면 될 것 같습니다.Also applies to: 51-66
src/main/java/team/wego/wegobackend/common/infrastructure/aws/image/ImageStorageService.java (2)
93-112: WebP 업로드 시 원본과 썸네일이 동일한 객체로 반환됩니다
uploadWithThumb의 WebP 분기에서:UploadedImage webp = new UploadedImage(webpKey, url); return new UploadedImagePair(webp, webp);와 같이 원본과 썸네일이 동일한 키/URL을 가리키고 있어, 클라이언트 입장에서는 “썸네일” 이 항상 원본과 같은 해상도의 이미지가 됩니다. 특히 리스트 화면에서 썸네일을 기대하는 경우, WebP 파일에 대해서는 성능 상 이점이 전혀 없어질 수 있습니다.
- WebP 에 대해서도 별도의 축소 썸네일을 만들거나
- 최소한 API 문서에 “WebP 의 경우 original/webpThumb 가 같은 리소스를 가리킨다” 고 명시하는 등 계약을 명확히 해 두는 것을 권장합니다.
151-164:toPairKey의 확장자 변환 규칙이 비대칭적이라, 썸네일 키를 넘겼을 때 원본 삭제가 누락될 수 있습니다현재
toPairKey는:
.webp→ 같은 베이스네임의.jpg.jpg/.jpeg/.png→ 같은 베이스네임의.webp로만 변환합니다. 이 상태에서:
- 원래 원본이
.png인 경우, 썸네일 키(...png.webp) 를deleteObjectWithWebpThumb에 넘기면:
- 썸네일(.webp)은 지워지지만
- 원본(.png)은
.jpg로 잘못 추론되어 삭제되지 않습니다.지금은 컨트롤러에서 “항상 원본 키를 넘긴다”는 전제가 있어 실질적인 오류는 없겠지만, 메서드 시그니처가
originalKey라는 이름 외에는 그런 제약을 표현하지 못하고 있고, 나중에 호출부가 바뀌면 쉽게 실수할 수 있는 구조입니다.
- 명시적으로 “항상 non-webp 원본 키만 받는다” 는 JavaDoc 을 추가하거나,
- 또는
toPairKey를 “어느 쪽 키를 넘겨도 상대 키를 정확히 찾는 양방향 매핑” 으로 재설계하는 방안을 고려해 볼 수 있습니다.Also applies to: 278-292
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/test/http/image/resources/img1.pngis excluded by!**/*.pngsrc/test/http/image/resources/img3.jpegis excluded by!**/*.jpeg
📒 Files selected for processing (8)
build.gradle(1 hunks)src/main/java/team/wego/wegobackend/common/infrastructure/aws/image/ImageStorageService.java(1 hunks)src/main/java/team/wego/wegobackend/common/infrastructure/aws/image/UploadedImagePair.java(1 hunks)src/main/java/team/wego/wegobackend/image/dto/UploadedImagePairResponse.java(1 hunks)src/main/java/team/wego/wegobackend/image/dto/UploadedImageResponse.java(1 hunks)src/main/java/team/wego/wegobackend/image/presentation/TestImageController.java(1 hunks)src/main/resources/application.yml(2 hunks)src/test/http/image/local-image-api.http(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/test/http/image/local-image-api.http
🔇 Additional comments (6)
build.gradle (1)
36-37: TwelveMonkeys ImageIO 의존성이 올바르게 선택되었습니다.TwelveMonkeys ImageIO는 WebP를 포함한 추가 이미지 형식에 대한 지원을 제공하며, 두 아티팩트(imageio-core와 imageio-webp)가 동일한 버전 3.12.0으로 정렬되어 있습니다. 필요한 JAR을 클래스 경로에 추가하면 ImageIO 레지스트리가 플러그인을 자동으로 사용 가능하게 함.
src/main/java/team/wego/wegobackend/common/infrastructure/aws/image/UploadedImagePair.java (1)
3-8: 이미지 쌍을 표현하는 DTO 레코드 설계가 단순·명확합니다원본과 썸네일(webp)을 분리해서 담는 구조가 이후 매핑/응답 DTO와도 잘 맞습니다. 별다른 수정 필요 없어 보입니다.
src/main/resources/application.yml (1)
5-8: 멀티파트 업로드 용량 제한이 요구사항과 잘 일치합니다단일 파일 10MB, 요청 전체 20MB로 설정되어 있어 애플리케이션 레벨 제약과 일관성이 있습니다. 운영 환경에서 여유를 더 줄 필요가 있으면 이 값만 조정하면 되어서 관리도 용이해 보입니다.
src/main/java/team/wego/wegobackend/image/dto/UploadedImageResponse.java (1)
6-13: 도메인 모델을 응답 DTO로 매핑하는 패턴이 깔끔합니다
UploadedImageResponse.from(UploadedImage)로 매핑을 일원화한 점이 좋습니다. 호출부에서image가 null 이 되지 않도록만 보장하면 충분해 보입니다.src/main/java/team/wego/wegobackend/image/dto/UploadedImagePairResponse.java (1)
6-17: 단일 DTO를 재사용한 쌍 매핑 구조가 좋습니다
UploadedImagePairResponse.from(UploadedImagePair)가 내부적으로UploadedImageResponse.from을 재사용해서 매핑 중복을 줄이고 있어 유지보수성 측면에서 좋습니다.src/main/java/team/wego/wegobackend/common/infrastructure/aws/image/ImageStorageService.java (1)
261-268: S3 키 포맷이 정의된 스펙{dir}/{timestamp}_{index}_{uuid}{ext}을 정확히 따릅니다
buildKey에서yyyyMMddHHmmss타임스탬프 + 업로드 순서 인덱스 + UUID + 확장자를 조합하는 방식이 PR 설명과 일치하고, 이름 충돌 가능성을 충분히 줄여 줍니다. 배치 업로드에서도index로 업로드 순서를 구분할 수 있어 추적성도 좋습니다.
| public UploadedImage uploadImage(String dir, MultipartFile file, int index) { | ||
| validateImageSize(file); | ||
| validateImageContentType(file); | ||
| validateExtension(file.getOriginalFilename()); | ||
|
|
||
| String originalFilename = file.getOriginalFilename(); | ||
| String extLower = extractExtension(originalFilename).toLowerCase(); | ||
|
|
||
| String key = buildKey(dir, originalFilename, index); | ||
| byte[] bytes; | ||
|
|
||
| try { | ||
| if (isWebp(extLower)) { | ||
| bytes = file.getBytes(); | ||
| } else { | ||
| bytes = resizeIfNeeded(file); | ||
| } | ||
| } catch (IOException e) { | ||
| throw new RuntimeException("이미지 업로드 중 오류가 발생했습니다.", e); | ||
| } | ||
|
|
||
| putToS3(key, bytes, file.getContentType()); | ||
| String url = publicEndpoint + "/" + key; | ||
|
|
||
| return new UploadedImage(key, url); | ||
| } |
There was a problem hiding this comment.
WebP 이미지에는 해상도(최대 가로·세로) 제한이 전혀 적용되지 않습니다
현재 로직상:
uploadImage/uploadWithThumb에서extLower가.webp인 경우resizeIfNeeded를 호출하지 않고file.getBytes()만 사용합니다.resizeIfNeeded안에서만maxWidth/maxHeight를 기준으로 리사이즈가 일어나므로, 결과적으로 WebP 파일은 가로·세로 2000px 제한을 전혀 받지 않습니다.
요구사항에서 “이미지 최대 크기(가로/세로) 2000px 제한”이 있었기 때문에, WebP에도 동일한 제약을 두려면 다음 중 하나가 필요합니다.
- WebP 를 처리할 수 있는 ImageIO/Thumbnailator 플러그인을 추가하고, WebP 에도
resizeIfNeeded와 동일한 제약을 적용하거나 - (플러그인 도입 전이라면) 당분간
.webp를ALLOWED_CONTENT_TYPES/ALLOWED_EXTENSIONS에서 빼서, 제약을 지킬 수 있는 포맷만 받도록 정책을 명확히 하는 방안
현 상태로는 WebP만 예외적으로 해상도 제한을 받지 않아, 운영 시 예상치 못한 초고해상도 WebP 이미지가 들어올 수 있습니다.
Also applies to: 93-130, 203-232
🤖 Prompt for AI Agents
In
src/main/java/team/wego/wegobackend/common/infrastructure/aws/image/ImageStorageService.java
around lines 57-82 (and similarly at 93-130, 203-232), WebP files are bypassing
resizeIfNeeded so the 2000px max dimension rule is not enforced; change the flow
to treat WebP the same as other images by removing the special-case branch and
always calling resizeIfNeeded (i.e., always process file through the resizing
pipeline), and if ImageIO/resize cannot read WebP, throw a clear validation
error and update ALLOWED_CONTENT_TYPES/ALLOWED_EXTENSIONS or the validation
logic to reflect supported formats; alternatively, if you prefer the temporary
policy approach, explicitly remove ".webp" and its content type from allowed
lists and return a validation error for WebP uploads until a WebP-capable
ImageIO/thumbnailing plugin is added — apply the same fix to the other
referenced line ranges.
| String webpKey = dir + "/" + baseName + ".webp"; | ||
| byte[] webpBytes = createThumbJpeg(file); | ||
| putToS3(webpKey, webpBytes, "image/jpeg"); | ||
| String webpUrl = publicEndpoint + "/" + webpKey; | ||
| UploadedImage webp = new UploadedImage(webpKey, webpUrl); |
There was a problem hiding this comment.
썸네일 키 확장자(.webp)와 실제 이미지 포맷/Content‑Type이 서로 맞지 않습니다
-
썸네일 업로드 시:
String webpKey = dir + "/" + baseName + ".webp"; byte[] webpBytes = createThumbJpeg(file); putToS3(webpKey, webpBytes, "image/jpeg");
-
createThumbJpeg는outputFormat("jpg")로 JPEG 바이너리를 생성하고 있습니다.
즉, S3 상에 JPEG 이미지를 저장하면서 객체 키는 .webp 확장자를 사용하고 있고, HTTP Content-Type 은 image/jpeg 입니다. 이는:
- 키만 보고 WebP 로 오인할 수 있고
- CDN·클라이언트 라이브러리 일부가 확장자를 근거로 처리할 경우 혼란을 줄 수 있으며
- PR 설명의 “WebP 썸네일” 과도 의미가 어긋납니다.
정책을 명확히 정리하는 쪽이 좋습니다.
- 실제로 WebP 썸네일을 만들고 싶다면:
outputFormat("webp")+contentType = "image/webp"로 맞추고, 인프라에 WebP 지원이 준비되어 있는지 확인하거나 - JPEG 썸네일로 가는 것이 의도라면: 키 확장자를
.jpg로 변경하고, 변수명(webpKey,webpThumb등)과toPairKey의 매핑 규칙도.jpg중심으로 정리하는 편이 더 일관됩니다.
지금처럼 “키는 .webp, 내용은 JPEG” 인 상태는 장기적으로 디버깅·운영 시 혼란을 키울 가능성이 높습니다.
Also applies to: 234-244, 278-292
🤖 Prompt for AI Agents
In
src/main/java/team/wego/wegobackend/common/infrastructure/aws/image/ImageStorageService.java
around lines 119-123 (also review similar blocks at 234-244 and 278-292): the
thumbnail key/variable names use a .webp extension while the code generates and
uploads JPEG bytes with Content-Type "image/jpeg", causing a mismatch; either
produce real WebP thumbnails (use outputFormat("webp") and set contentType to
"image/webp" and ensure infra supports WebP) or switch keys/variable
names/extensions to .jpg and keep contentType "image/jpeg" (also update any
mapping like toPairKey and variable names such as webpKey/webpThumb to reflect
the chosen format consistently).
|
@LimdaeIl 전체적으로 LGTM입니다~ |
네 맞습니다! 근데 지금 이미지 최적화 하다보니까... 어제부터 지금까지 삽질 좀 해서 다시 아예 새롭게 만들고 있습니다..! 기능 자체는 모두 동일한데, 최종 정리되면 이거 디스코드로 말씀 다시 드리겠습니다..! ㅜㅜ 이미지 최적화 깊게 해보려다가 엉뚱한 길로 빠져서 해메서 빨리 끝내보겠습니다 |
코드가 보기 불편하고, 이미지 생성 + 썸네일 생성 처럼 복잡한 로직이 한 개의 api 에 존재하는 것을 제거했습니다. 그리고 image를 더이상 common에 두는 것 보다, 별도의 image 패키지로 분리해서 사용할 수 있도록 구조를 수정했습니다.
webp 변환과 이미지 크기 조정에서 thumbnailator, twelvemonkeys 두 개의 라이브러리 충돌로 의도와 다른 동작을 수행하는 것을 확인했습니다. 그래서 twelvemonkeys 의 webp를 제거하고, org.sejda.imageio:webp-imageio를 통해 webp 를 변화하도록 했습니다.
image 목적으로만 사용되는 S3를 image 패키지 하위로 이동했습니다.
이미지 경로는 buildKey 메서드에서,
return dir + "/" + timestamp + "_" + index + "_" + uuid + extension;
형식으로 저장됩니다. 이때 dir 경로는 S3 디렉토리에 해당합니다. 이미지 기능 구현한 기능은 다음과 같습니다.
1. 한 개의 원본 이미지 저장
2. 여러 개의 원본 이미지 저장
3. 한 개의 이미지(jpg, png, jeng 등) -> webp 이미지로 변환 후 저장
4. 여러 개의 이미지(jpg, png, jeng 등) -> 모두 webp 이미지로 변환 후 저장
4. 한 개의 이미지 삭제
5. 여러 개의 이미지 삭제
6. 이미지 크기를 thumb-max-width 기준으로 맞춰서 최적화하여 저장
(600 x 600 이지만, 가로 기준으로 thumbnailator로 최적화하여 600 x 400, 600 x 280 등이 될 수 있어요.)
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
src/main/java/team/wego/wegobackend/image/infrastructure/S3Config.java (1)
17-21: S3 자격 증명 값 검증 부재환경 변수에서 주입받는
accessKeyId와secretAccessKey값이 비어있거나 null인 경우를 검증하지 않습니다. 이 값들이 설정되지 않으면 런타임에 불명확한 오류가 발생할 수 있습니다.
@Value어노테이션에 기본값 없이 필수 속성으로 처리되는지, 또는 애플리케이션 시작 시 검증이 이루어지는지 확인해주세요.src/main/java/team/wego/wegobackend/image/application/service/ImageUploadService.java (2)
140-146: 오류 메시지의 바이트 단위 표시오류 메시지가 바이트 단위로 표시되어 사용자 친화적이지 않습니다. 10MB 제한의 경우 "10485760 bytes"로 표시됩니다.
206-215: PUBLIC_READ ACL 사용에 대한 보안 검토업로드된 객체에
PUBLIC_READACL이 설정되어 URL을 아는 누구나 접근할 수 있습니다. 의도된 동작인지 확인이 필요합니다.
🧹 Nitpick comments (3)
src/main/java/team/wego/wegobackend/image/presentation/ImageController.java (1)
24-34: RESTful 응답 상태 코드 개선 권장POST 요청은 201 Created, DELETE 요청은 204 No Content를 반환하는 것이 RESTful 관례입니다.
+import org.springframework.http.HttpStatus; +import org.springframework.web.bind.annotation.ResponseStatus; + @ResponseStatus(HttpStatus.CREATED) @PostMapping( value = "/original", consumes = MediaType.MULTIPART_FORM_DATA_VALUE )삭제 엔드포인트에도 동일하게 적용:
@ResponseStatus(HttpStatus.NO_CONTENT) @DeleteMapping("/one") public void deleteOne(@RequestParam("key") String key) { // ... }src/main/java/team/wego/wegobackend/image/application/service/ImageUploadService.java (2)
249-261: ByteArrayOutputStream 리소스 관리
ByteArrayOutputStream이 try-with-resources 블록 밖에서 생성되었습니다.ByteArrayOutputStream은 외부 리소스를 보유하지 않아 실제 누수는 없지만, 코드 일관성과 모범 사례를 위해 try-with-resources 사용을 권장합니다.- ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream(); - String formatName = getFormatName(file.getOriginalFilename()); - - Thumbnails.of(resized) - .size(resized.getWidth(), resized.getHeight()) - .outputFormat(formatName) - .toOutputStream(byteArrayOutputStream); - - if (byteArrayOutputStream.size() == 0) { - throw new IllegalStateException(errorPrefix + "에 실패했습니다. format=" + formatName); - } - - return byteArrayOutputStream.toByteArray(); + String formatName = getFormatName(file.getOriginalFilename()); + try (ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream()) { + Thumbnails.of(resized) + .size(resized.getWidth(), resized.getHeight()) + .outputFormat(formatName) + .toOutputStream(byteArrayOutputStream); + + if (byteArrayOutputStream.size() == 0) { + throw new IllegalStateException(errorPrefix + "에 실패했습니다. format=" + formatName); + } + + return byteArrayOutputStream.toByteArray(); + }
134-138: 다중 삭제 시 S3 배치 삭제 사용 권장현재 구현은 각 키마다 개별 API 호출을 수행합니다. S3는 단일 요청으로 최대 1000개 객체를 삭제할 수 있는 배치 삭제를 지원합니다.
public void deleteAll(List<String> keys) { - for (String key : keys) { - delete(key); - } + if (keys.isEmpty()) { + return; + } + List<ObjectIdentifier> objectIds = keys.stream() + .map(key -> ObjectIdentifier.builder().key(key).build()) + .toList(); + + DeleteObjectsRequest deleteRequest = DeleteObjectsRequest.builder() + .bucket(bucket) + .delete(Delete.builder().objects(objectIds).build()) + .build(); + + s3Client.deleteObjects(deleteRequest); }추가 import 필요:
import software.amazon.awssdk.services.s3.model.Delete; import software.amazon.awssdk.services.s3.model.DeleteObjectsRequest; import software.amazon.awssdk.services.s3.model.ObjectIdentifier;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
build.gradle(1 hunks)src/main/java/team/wego/wegobackend/image/application/dto/ImageFileResponse.java(1 hunks)src/main/java/team/wego/wegobackend/image/application/service/ImageUploadService.java(1 hunks)src/main/java/team/wego/wegobackend/image/domain/ImageFile.java(1 hunks)src/main/java/team/wego/wegobackend/image/infrastructure/S3Config.java(1 hunks)src/main/java/team/wego/wegobackend/image/presentation/ImageController.java(1 hunks)src/test/http/image/image-api.http(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- build.gradle
🔇 Additional comments (4)
src/main/java/team/wego/wegobackend/image/domain/ImageFile.java (1)
3-7: LGTM!간결한 도메인 레코드입니다.
key와url을 담는 불변 데이터 캐리어로 적절합니다.src/main/java/team/wego/wegobackend/image/application/dto/ImageFileResponse.java (1)
5-13: LGTM!도메인 객체에서 DTO로 변환하는 정적 팩토리 메서드 패턴이 잘 적용되어 있습니다.
src/main/java/team/wego/wegobackend/image/application/service/ImageUploadService.java (2)
114-128: 썸네일 업로드 시 Content-Type 일관성 확인
uploadThumb는file.getContentType()을 S3에 전달하지만,resizeToThumb내부에서getFormatName()에 의해 결정된 포맷으로 이미지가 인코딩됩니다. 대부분의 경우 일치하지만,jpeg와jpg처리 등에서 불일치가 발생할 수 있습니다.원본 Content-Type과 실제 출력 포맷이 항상 일치하는지 테스트를 통해 확인해주세요.
268-279: WebP support is properly configured and does not require verificationThe project includes both
com.twelvemonkeys.imageio:imageio-core:3.12.0andorg.sejda.imageio:webp-imageio:0.1.6dependencies. There are dedicatedconvertToWebp(),uploadAsWebp(), anduploadAllAsWebp()methods with explicit WebP conversion and error handling, indicating WebP write support is intentionally implemented. ThegetFormatName()method correctly returns "webp" for .webp files.
| public ImageFileResponse uploadOriginal( | ||
| @RequestParam("dir") String dir, | ||
| @RequestPart("file") MultipartFile file | ||
| ) { | ||
| ImageFile image = imageUploadService.uploadOriginal(dir, file, 0); | ||
| return ImageFileResponse.from(image); | ||
| } |
There was a problem hiding this comment.
dir 파라미터 경로 순회(Path Traversal) 취약점
dir 파라미터가 검증 없이 S3 키 생성에 직접 사용됩니다. 악의적인 사용자가 ../ 또는 절대 경로를 포함한 값을 전달하면 의도하지 않은 S3 경로에 파일을 저장할 수 있습니다.
서비스 레이어에서 dir 값에 대한 검증을 추가하세요:
private void validateDir(String dir) {
if (dir == null || dir.isBlank()) {
throw new IllegalArgumentException("dir은 필수입니다.");
}
if (dir.contains("..") || dir.startsWith("/")) {
throw new IllegalArgumentException("잘못된 디렉토리 경로입니다.");
}
}🤖 Prompt for AI Agents
In src/main/java/team/wego/wegobackend/image/presentation/ImageController.java
around lines 28 to 34, the dir request parameter is passed directly to the
service causing a potential path-traversal when building S3 keys; add a
validation step before calling imageUploadService.uploadOriginal that ensures
dir is non-null/non-blank and does not contain ".." or start with "/" (and throw
an IllegalArgumentException with a clear message on failure), or move this
validateDir logic into the service layer and invoke it there so only validated
paths reach S3 key creation.
| @DeleteMapping("/one") | ||
| public void deleteOne(@RequestParam("key") String key) { | ||
| imageUploadService.delete(key); | ||
| } | ||
|
|
||
| @DeleteMapping | ||
| public void deleteMany(@RequestParam("keys") List<String> keys) { | ||
| imageUploadService.deleteAll(keys); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the full ImageController class
fd -t f "ImageController.java" --exec wc -l {} \;Repository: WeGo-Together/WeGo_BackEnd
Length of output: 196
🏁 Script executed:
# Read the full ImageController
fd -t f "ImageController.java" --exec cat -n {} \;Repository: WeGo-Together/WeGo_BackEnd
Length of output: 3976
🏁 Script executed:
# Search for @PreAuthorize, @Secured, or other security annotations
rg -t java "@PreAuthorize|@Secured|@PermitAll|@EnableWebSecurity" --max-count=20Repository: WeGo-Together/WeGo_BackEnd
Length of output: 52
🏁 Script executed:
# Find the imageUploadService implementation
fd -t f "ImageUploadService.java" --exec cat -n {} \;Repository: WeGo-Together/WeGo_BackEnd
Length of output: 11485
🏁 Script executed:
# Look for any global security configuration
fd -t f "SecurityConfig.java|WebSecurityConfig.java|SecurityConfiguration.java" --exec cat -n {} \;Repository: WeGo-Together/WeGo_BackEnd
Length of output: 52
🏁 Script executed:
# Check for any interceptors or filters that might handle authorization
fd -t f "Filter.java|Interceptor.java" -path "*/wego*" --exec cat -n {} \;Repository: WeGo-Together/WeGo_BackEnd
Length of output: 238
🏁 Script executed:
# Search for authorization checks in service layer for delete operations
rg -t java "delete|Delete" -A 3 -B 3 | head -100Repository: WeGo-Together/WeGo_BackEnd
Length of output: 3756
🏁 Script executed:
# Search for request filters, interceptors, or AOP security
rg -i "filter|interceptor|aspect" --max-count=30 src/main/java | grep -i "security\|auth" | head -20Repository: WeGo-Together/WeGo_BackEnd
Length of output: 52
🏁 Script executed:
# Look for any SecurityContextHolder or Authentication usage
rg "SecurityContextHolder|Authentication|Principal" src/main/javaRepository: WeGo-Together/WeGo_BackEnd
Length of output: 52
🏁 Script executed:
# Check for any security-related beans or configurations in XML or properties
fd -t f "application.yml\|application.yaml\|web.xml" --exec cat {} \;Repository: WeGo-Together/WeGo_BackEnd
Length of output: 52
🏁 Script executed:
# Search for any Spring Security configuration or beans
rg "@EnableWebSecurity|@Configuration|SecurityFilterChain" --max-count=20Repository: WeGo-Together/WeGo_BackEnd
Length of output: 155
🏁 Script executed:
# Search for tests related to delete operations
fd -t f "ImageControllerTest.java\|ImageUploadServiceTest.java" --exec cat {} \;Repository: WeGo-Together/WeGo_BackEnd
Length of output: 52
🏁 Script executed:
# Look for README or documentation
fd -t f "README.md\|API.md\|DOCUMENT.md" --exec head -100 {} \;Repository: WeGo-Together/WeGo_BackEnd
Length of output: 52
🏁 Script executed:
# Check for any comments or documentation in the controller about authorization
rg -B 5 "@DeleteMapping" src/main/java/team/wego/wegobackend/image/presentation/ImageController.javaRepository: WeGo-Together/WeGo_BackEnd
Length of output: 374
Delete endpoints require authentication and authorization checks
The deleteOne and deleteMany endpoints are publicly accessible without any authentication or authorization mechanism. Any caller can delete arbitrary images by providing the key parameter. Implement authentication checks and verify that the requester has permission to delete the image (e.g., owns the associated content) before executing the delete operation.
🤖 Prompt for AI Agents
In src/main/java/team/wego/wegobackend/image/presentation/ImageController.java
around lines 73 to 81, the deleteOne and deleteMany endpoints are currently
publicly accessible; change them to require an authenticated user and enforce
authorization checks before calling the service. Modify the methods to accept
the authenticated principal (or user id via @AuthenticationPrincipal or
SecurityContext) and call an authorization helper in the service/repo to verify
the current user is allowed to delete the specified image(s) (for deleteMany,
verify permission for each key or reject the request if any key is not owned);
if authorization fails return/throw 403 Forbidden. Also ensure controller
methods are annotated or secured according to the project’s security
configuration (e.g., @PreAuthorize("isAuthenticated()") or the equivalent) and
update ImageUploadService to expose a checkOwnership/deleteIfOwned helper used
by the controller.
| Content-Disposition: form-data; name="file"; filename="big-image.jpg" | ||
| Content-Type: image/jpeg | ||
|
|
||
| < ./test-webp2.webp | ||
| --boundary-- |
There was a problem hiding this comment.
파일 경로 불일치 및 Content-Type 오류
두 가지 문제가 있습니다:
Content-Type: image/jpeg이지만 실제 파일은.webp파일입니다.- 파일 경로가
./test-webp2.webp로, 다른 테스트들의./resources/경로와 일치하지 않습니다.
--boundary
-Content-Disposition: form-data; name="file"; filename="big-image.jpg"
-Content-Type: image/jpeg
+Content-Disposition: form-data; name="file"; filename="test-webp2.webp"
+Content-Type: image/webp
-< ./test-webp2.webp
+< ./resources/test-webp2.webp📝 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.
| Content-Disposition: form-data; name="file"; filename="big-image.jpg" | |
| Content-Type: image/jpeg | |
| < ./test-webp2.webp | |
| --boundary-- | |
| Content-Disposition: form-data; name="file"; filename="test-webp2.webp" | |
| Content-Type: image/webp | |
| < ./resources/test-webp2.webp | |
| --boundary-- |
🤖 Prompt for AI Agents
In src/test/http/image/image-api.http around lines 80 to 84, the multipart
section incorrectly labels the uploaded file as Content-Type: image/jpeg and
references ./test-webp2.webp at the repo root; change the Content-Type to
image/webp to match the .webp file and update the file path to the standardized
test resources directory (e.g. ./resources/test-webp2.webp) so the test reads
the correct file and MIME type.
이미지 업로드를 위해 제공된 디렉터리 경로가 유효한지 유효성 검사를 추가하여 확인합니다. 이를 통해 잠재적인 보안 취약점을 방지하고 데이터 무결성을 보장합니다. 디렉토리 경로의 유효성을 검사하여 경로 순회 취약점을 방지하고 정의된 기준을 충족하는지 확인합니다.
github-project-automation
bot
moved this from In progress
to Done
in WeGo-Together Backend
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/main/java/team/wego/wegobackend/image/application/service/ImageUploadService.java (2)
145-147: 오류 메시지에 바이트 단위 표시 개선을 권장합니다.사용자 친화적이지 않은 바이트 단위(예: 10485760 bytes) 대신 MB 단위로 표시하는 것이 좋습니다.
이전 리뷰에서 이미 제안된 내용입니다. 다음과 같이 개선할 수 있습니다:
private void validateImageSize(MultipartFile file) { if (file.getSize() > maxSizeBytes) { + double maxSizeMB = maxSizeBytes / (1024.0 * 1024.0); throw new IllegalArgumentException( - "이미지 크기가 너무 큽니다. 최대 " + maxSizeBytes + " bytes 까지만 허용됩니다." + String.format("이미지 크기가 너무 큽니다. 최대 %.1f MB 까지만 허용됩니다.", maxSizeMB) ); } }
209-218: S3 객체의 PUBLIC_READ ACL 설정에 대한 보안 검토가 필요합니다.업로드된 모든 객체가 공개적으로 접근 가능하게 설정됩니다. 사용 사례에 따라 의도된 동작일 수 있으나, 더 제한적인 ACL과 pre-signed URL을 사용하는 것이 보안상 더 안전합니다. 최소한 이 동작이 문서화되거나 설정 가능하도록 하는 것을 권장합니다.
이전 리뷰에서 이미 제기된 내용입니다. 다음과 같이 ACL을 설정 가능하게 만들 수 있습니다:
+ @Value("${aws.s3.default-acl:PUBLIC_READ}") + private String defaultAcl; + private void putToS3(String key, byte[] bytes, String contentType) { PutObjectRequest putObjectRequest = PutObjectRequest.builder() .bucket(bucket) .key(key) .contentType(contentType) - .acl(ObjectCannedACL.PUBLIC_READ) + .acl(ObjectCannedACL.valueOf(defaultAcl)) .build(); s3Client.putObject(putObjectRequest, RequestBody.fromBytes(bytes)); }
🧹 Nitpick comments (2)
src/main/java/team/wego/wegobackend/image/application/service/ImageUploadService.java (2)
137-141: S3 일괄 삭제 최적화를 권장합니다.현재 구현은 순차적으로 개별 삭제를 수행하므로, 다수의 파일 삭제 시 레이턴시와 API 호출 수가 증가합니다. AWS S3 SDK의
DeleteObjectsRequest를 사용하면 최대 1000개의 객체를 단일 요청으로 삭제할 수 있습니다.다음과 같이 일괄 삭제 API를 활용하는 것을 권장합니다:
+import software.amazon.awssdk.services.s3.model.Delete; +import software.amazon.awssdk.services.s3.model.DeleteObjectsRequest; +import software.amazon.awssdk.services.s3.model.ObjectIdentifier; + public void deleteAll(List<String> keys) { - for (String key : keys) { - delete(key); - } + if (keys.isEmpty()) { + return; + } + + List<ObjectIdentifier> objectIdentifiers = keys.stream() + .map(key -> ObjectIdentifier.builder().key(key).build()) + .toList(); + + Delete delete = Delete.builder().objects(objectIdentifiers).build(); + DeleteObjectsRequest request = DeleteObjectsRequest.builder() + .bucket(bucket) + .delete(delete) + .build(); + + s3Client.deleteObjects(request); }
234-234: TODO 주석의 공통 예외 처리 구현을 지원할 수 있습니다.공통 예외 처리 로직 구현을 통해
try-catch중복을 제거하고 싶으시다면, Spring의@ControllerAdvice나 커스텀 예외 클래스를 활용한 구현을 도와드릴 수 있습니다.이 TODO를 해결하기 위한 구현 코드를 생성하거나 새로운 이슈로 등록하시겠습니까?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/team/wego/wegobackend/image/application/service/ImageUploadService.java(1 hunks)
🔇 Additional comments (8)
src/main/java/team/wego/wegobackend/image/application/service/ImageUploadService.java (8)
1-26: 클래스 구조 및 의존성 설정이 적절합니다.AWS SDK v2와 Thumbnailator 라이브러리를 사용한 구성이 적절하며, Lombok을 통한 생성자 주입 패턴도 올바르게 적용되었습니다.
28-62: 설정 상수 및 필드 선언이 적절합니다.허용된 콘텐츠 타입과 확장자가 불변 리스트로 정의되어 있으며,
@Value를 통한 외부 설정 주입 방식도 적절합니다.
64-88: 원본 이미지 업로드 로직이 올바르게 구현되었습니다.검증, 키 생성, 리사이즈, S3 업로드 흐름이 명확하게 구현되어 있습니다.
90-114: WebP 변환 업로드 로직이 적절합니다.WebP 변환 로직이 별도 메서드로 분리되어 있어 유지보수성이 좋습니다.
169-190: 키 생성 및 확장자 추출 로직이 적절합니다.S3 키 형식이 PR 명세(
{dir}/{timestamp}_{index}_{uuid}{ext})와 일치하며, 유일성 보장을 위한 UUID 사용도 적절합니다.
192-207: WebP 변환 로직이 안전하게 구현되었습니다.
try-with-resources를 사용하여 리소스를 적절히 관리하고 있으며, 변환 실패를 감지하는 검증 로직도 포함되어 있습니다.
271-282: 이미지 포맷 이름 매핑 로직이 적절합니다.
jpeg를jpg로 정규화하는 로직과 기본값 처리가 올바르게 구현되어 있습니다.
284-300: 디렉토리 경로 검증 로직이 포괄적입니다.경로 순회 공격 방지(
..검사), 유효한 문자 제한, 형식 검증 등 보안을 고려한 검증이 잘 구현되어 있습니다.
| ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream(); | ||
| String formatName = getFormatName(file.getOriginalFilename()); | ||
|
|
||
| Thumbnails.of(resized) | ||
| .size(resized.getWidth(), resized.getHeight()) | ||
| .outputFormat(formatName) | ||
| .toOutputStream(byteArrayOutputStream); | ||
|
|
||
| if (byteArrayOutputStream.size() == 0) { | ||
| throw new IllegalStateException(errorPrefix + "에 실패했습니다. format=" + formatName); | ||
| } | ||
|
|
||
| return byteArrayOutputStream.toByteArray(); |
There was a problem hiding this comment.
ByteArrayOutputStream 리소스 누수 위험이 있습니다.
Line 252의 ByteArrayOutputStream이 try-with-resources 구문 외부에서 생성되어 예외 발생 시 리소스가 해제되지 않을 수 있습니다.
다음과 같이 수정하여 리소스를 안전하게 관리하는 것을 권장합니다:
BufferedImage resized = Thumbnails.of(originalImage)
.size(targetMaxWidth, targetMaxHeight)
.asBufferedImage();
- ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream();
String formatName = getFormatName(file.getOriginalFilename());
- Thumbnails.of(resized)
- .size(resized.getWidth(), resized.getHeight())
- .outputFormat(formatName)
- .toOutputStream(byteArrayOutputStream);
-
- if (byteArrayOutputStream.size() == 0) {
- throw new IllegalStateException(errorPrefix + "에 실패했습니다. format=" + formatName);
- }
-
- return byteArrayOutputStream.toByteArray();
+ try (ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream()) {
+ Thumbnails.of(resized)
+ .size(resized.getWidth(), resized.getHeight())
+ .outputFormat(formatName)
+ .toOutputStream(byteArrayOutputStream);
+
+ if (byteArrayOutputStream.size() == 0) {
+ throw new IllegalStateException(errorPrefix + "에 실패했습니다. format=" + formatName);
+ }
+
+ return byteArrayOutputStream.toByteArray();
+ }
} catch (IOException e) {
throw new RuntimeException(errorPrefix + " 중 오류가 발생했습니다.", e);
}📝 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.
| ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream(); | |
| String formatName = getFormatName(file.getOriginalFilename()); | |
| Thumbnails.of(resized) | |
| .size(resized.getWidth(), resized.getHeight()) | |
| .outputFormat(formatName) | |
| .toOutputStream(byteArrayOutputStream); | |
| if (byteArrayOutputStream.size() == 0) { | |
| throw new IllegalStateException(errorPrefix + "에 실패했습니다. format=" + formatName); | |
| } | |
| return byteArrayOutputStream.toByteArray(); | |
| BufferedImage resized = Thumbnails.of(originalImage) | |
| .size(targetMaxWidth, targetMaxHeight) | |
| .asBufferedImage(); | |
| String formatName = getFormatName(file.getOriginalFilename()); | |
| try (ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream()) { | |
| Thumbnails.of(resized) | |
| .size(resized.getWidth(), resized.getHeight()) | |
| .outputFormat(formatName) | |
| .toOutputStream(byteArrayOutputStream); | |
| if (byteArrayOutputStream.size() == 0) { | |
| throw new IllegalStateException(errorPrefix + "에 실패했습니다. format=" + formatName); | |
| } | |
| return byteArrayOutputStream.toByteArray(); | |
| } | |
| } catch (IOException e) { | |
| throw new RuntimeException(errorPrefix + " 중 오류가 발생했습니다.", e); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/team/wego/wegobackend/image/application/service/ImageUploadService.java
around lines 252 to 264, the ByteArrayOutputStream is created outside a
try-with-resources which can leak the stream if an exception occurs; wrap the
ByteArrayOutputStream creation and the Thumbnails.toOutputStream call in a
try-with-resources block so the stream is always closed, obtain the byte[] from
the stream inside that block (or copy it to a local variable before exiting),
and then perform the zero-size check and possible IllegalStateException after
safely acquiring the byte[].
2 participants
📝 Pull Request
📌 PR 종류
해당하는 항목에 체크해주세요.
✨ 변경 내용
S3에 이미지 등록, 삭제가 핵심 기능으로 최적화를 고려한 서비스 구현을 목표로 작성한 코드들 입니다.
이미지의 최종 키는
{dir}/{timestamp}_{index}_{uuid}{ext}형태입니다.애플리케이션 수준에서는 한 파일은 10MB까지, 요청 전체는 20MB까지 보낼 수 있도록 제한했습니다.
S3 접속을 위한 민감한 정보를 감추고, 저장될 이미지 파일 최대 용량 (바이트 단위)은 10MB입니다. 이미지 크기 가로와 세로 최대 픽셀 크기는 2000으로 제한합니다.
🔍 관련 이슈
🧪 테스트
변경된 기능에 대한 테스트 범위 또는 테스트 결과를 작성해주세요.
🚨 확인해야 할 사항 (Checklist)
PR을 제출하기 전에 아래 항목들을 확인해주세요.
🙋 기타 참고 사항
리뷰어가 참고하면 좋을 만한 추가 설명이 있다면 적어주세요.
Summary by CodeRabbit
릴리스 노트
✏️ Tip: You can customize this high-level summary in your review settings.