[BE] fix: .jpg 이외의 이미지도 업로드할 수 있게 수정 #605

takoyakimchi · 2024-10-02T05:34:35Z

이슈

close S3 업로드할 때, 파일의 확장자를 파싱하여 확장자에 맞게 업로드 #216

개발 사항

.jpg 이외의 이미지 업로드 가능
확장자 유지, 파일명은 UUID로 변경
- example.jpeg -> 6f4914d3-144d-4e0c-8859-d0b8ff7d0120.jpeg
이미지 파일이 아닌 경우 예외 발생
이미지 파일인지 판단하는 방법: MIME 타입이 image/로 시작하는지 판단한다 참고링크

전달 사항 (없으면 삭제해 주세요)

dev 환경에서 정상 작동 확인했습니다.

github-actions · 2024-10-02T05:36:44Z

Test Results

44 files + 39 44 suites +39 18s ⏱️ +17s
190 tests +147 190 ✅ +147 0 💤 ±0 0 ❌ ±0
191 runs +148 191 ✅ +148 0 💤 ±0 0 ❌ ±0

Results for commit d8cd996. ± Comparison against base commit 1d8f222.

This pull request removes 43 and adds 190 tests. Note that renamed tests count towards both.

com.happy.friendogly.presentation.ui.club.detail.ClubDetailViewModelTest ‑ 모임 상세 로드에 실패하면 사용자에게 실패를 알리는 이벤트를 발생시킨다()
com.happy.friendogly.presentation.ui.club.detail.ClubDetailViewModelTest ‑ 모임 상세가 로드된다()
com.happy.friendogly.presentation.ui.club.detail.ClubDetailViewModelTest ‑ 모임 참여에 성공하면 사용자에게 참여 성공을 알리는 이벤트가 발생된다()
com.happy.friendogly.presentation.ui.club.detail.ClubDetailViewModelTest ‑ 모임 참여할 수 없는 경우 사용자에게 참여 실패를 알리는 이벤트가 발생한다()
com.happy.friendogly.presentation.ui.club.list.ClubListViewModelTest ‑ 등록한 주소가 없는 경우 사용자에게 주소 등록을 요구하는 화면을 표시한다()
com.happy.friendogly.presentation.ui.club.list.ClubListViewModelTest ‑ 등록한 주소의 모임 리스트가 로드된다()
com.happy.friendogly.presentation.ui.club.list.ClubListViewModelTest ‑ 모임 리스트 로드에 실패하면 사용자에게 에러 발생을 나타내는 화면을 표시한다()
com.happy.friendogly.presentation.ui.club.list.ClubListViewModelTest ‑ 적합한 모임이 없는 경우 사용자에게 모임이 없다는 것을 나타내는 화면을 표시한다()
com.happy.friendogly.presentation.ui.mypage.MyPageViewModelTest ‑ 내가 방장인 모임 이동 이벤트가 발생한다()
com.happy.friendogly.presentation.ui.mypage.MyPageViewModelTest ‑ 내가 참여한 모임 이동 이벤트가 발생한다()
…

com.happy.friendogly.DataSourceRoutingTest ‑ readOnly 트랜잭션이면 redaer 데이터소스가 바운딩된다.
com.happy.friendogly.DataSourceRoutingTest ‑ 쓰기전용 트랜잭션이면 Writer 데이터소스가 바운딩된다.
com.happy.friendogly.chat.controller.ChatMessageControllerTest ‑ 두 시점 사이의 채팅 메시지를 조회할 수 있다.
com.happy.friendogly.chat.service.ChatCommandServiceTest ‑ 채팅 메시지를 DB에 저장한다.
com.happy.friendogly.chat.service.ChatCommandServiceTest ‑ 채팅 입장 메시지를 DB에 저장한다.
com.happy.friendogly.chat.service.ChatCommandServiceTest ‑ 채팅 퇴장 메시지를 DB에 저장한다.
com.happy.friendogly.chat.service.ChatQueryServiceTest ‑ since 시간과 until 시간 사이의 (포함 X) 채팅 내역을 조회할 수 있다.
com.happy.friendogly.chat.service.ChatQueryServiceTest ‑ 채팅 메시지 시간 범위 조회에서, 시간 범위가 잘못된 경우 예외가 발생한다.
com.happy.friendogly.chat.service.ChatQueryServiceTest ‑ 채팅방에 들어가 있지 않은 Member는 채팅 내역을 조회할 수 없다.
com.happy.friendogly.chat.service.ChatRoomCommandServiceTest ‑ 1대1 채팅방 저장 과정에서, 이미 채팅방이 존재하면 기존 채팅방 ID를 반환한다.
…

♻️ This comment has been updated with latest results.

ehtjsv2

코멘트 하나 확인부탁드립니다~

ehtjsv2 · 2024-10-02T07:03:40Z

backend/src/main/java/com/happy/friendogly/infra/S3StorageManager.java

+        int dotIndex = fileName.lastIndexOf(".");
+
+        if (dotIndex == -1) {
+            return "";


확장자가 없는 파일이 s3에 업로드 될 수도 있는 걸 의도한건가요??

넵 맞아요~~~ + outofbound 방지입니다

확장자 없는 경우 포함하고 이미지에 관련한 확장자가 아닐 경우에도 예외 주면서 막아야되지 않나요?

content type으로 막고 있슴다

이미지 파일인지 판단하는 방법: MIME 타입이 image/로 시작하는지 판단한다 참고링크

요청 헤더는 충분히 조작될 수 있지 않을까요?

multipart 구조를 보면 각각의 파트는 고유의 Content-Type을 가집니다.

만약 악성 파일의 Content-Type을 image/... 로 전송하면 현재 구조에서는 S3 업로드를 허용하게 될 것 같아요. Content-Type만 믿으면 안될 것 같고, 추가적인 검증이 필요한 부분 같습니다.

물론 어떻게 검증할지는 좀 더 생각해봐야 하겠지만요!

https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/util/StringUtils.html#getFilenameExtension(java.lang.String)

추가로 StringUtils.getFilenameExtension() 메서드를 사용하면 확장자를 추출하는 현재 메서드는 제거할 수 있겠습니다!

StringUtils.getFilenamExtension(filename);

요청 헤더는 충분히 조작될 수 있지 않을까요?

이건 당장 생각이 나지 않아서 별도 이슈에서 작업하면 좋겠네요. #620

추가로 StringUtils.getFilenameExtension() 메서드를 사용하면 확장자를 추출하는 현재 메서드는 제거할 수 있겠습니다!

반영 완료

jimi567

확장자 부분 잘 이해가 안갔어요. 제가 놓친 도메인 규칙이 이씅ㄹ까용?

jimi567 · 2024-10-02T13:09:46Z

backend/src/main/java/com/happy/friendogly/infra/S3StorageManager.java

+        int dotIndex = fileName.lastIndexOf(".");
+
+        if (dotIndex == -1) {
+            return "";


확장자 없는 경우 포함하고 이미지에 관련한 확장자가 아닐 경우에도 예외 주면서 막아야되지 않나요?

J-I-H-O · 2024-10-02T13:10:09Z

backend/src/main/java/com/happy/friendogly/infra/S3StorageManager.java

-                .contentType("image/jpg")
+                .contentType(contentType)


https://www.iana.org/assignments/media-types/media-types.xhtml#image

순수한 궁금증.. image/jpg 라는 MIME 타입은 없는데 그동안 어떻게 동작하고 있던걸까요..?
자동으로 image/jpeg로 변환되는걸까요?

그러게요 ㄷ ㄷ ㄷ

ehtjsv2

확장자가 없이 이미지가 오는 경우는 거의 없다고 보고, 이미지에 관련된 확장자가 아닐 경우도 MIME로 막고있는 것으로 보아 approve합니다

추정)MIME의 타입은 파일의 확장자로 자동으로 된다!?

J-I-H-O · 2024-10-06T08:15:24Z

backend/src/main/java/com/happy/friendogly/infra/S3StorageManager.java

+        int dotIndex = fileName.lastIndexOf(".");
+
+        if (dotIndex == -1) {
+            return "";


요청 헤더는 충분히 조작될 수 있지 않을까요?

multipart 구조를 보면 각각의 파트는 고유의 Content-Type을 가집니다.

만약 악성 파일의 Content-Type을 image/... 로 전송하면 현재 구조에서는 S3 업로드를 허용하게 될 것 같아요. Content-Type만 믿으면 안될 것 같고, 추가적인 검증이 필요한 부분 같습니다.

물론 어떻게 검증할지는 좀 더 생각해봐야 하겠지만요!

J-I-H-O · 2024-10-06T08:17:14Z

backend/src/main/java/com/happy/friendogly/infra/S3StorageManager.java

+        int dotIndex = fileName.lastIndexOf(".");
+
+        if (dotIndex == -1) {
+            return "";


https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/util/StringUtils.html#getFilenameExtension(java.lang.String)

추가로 StringUtils.getFilenameExtension() 메서드를 사용하면 확장자를 추출하는 현재 메서드는 제거할 수 있겠습니다!

StringUtils.getFilenamExtension(filename);

ehtjsv2

👍

takoyakimchi added 4 commits October 2, 2024 14:13

fix: .jpg 이외의 이미지 파일도 업로드할 수 있도록 수정

3ce8e90

feat: 이미지 테스트용 컨트롤러 작성

6b84918

remove: 테스트용 컨트롤러 제거

7feb662

refactor: 예외 메시지 구체화

c157dae

takoyakimchi added 🛠️ refactor 리팩터링 🖥 backend backend labels Oct 2, 2024

takoyakimchi added this to the Sprint6 milestone Oct 2, 2024

takoyakimchi self-assigned this Oct 2, 2024

takoyakimchi requested review from ehtjsv2, jimi567 and J-I-H-O as code owners October 2, 2024 05:34

ehtjsv2 reviewed Oct 2, 2024

View reviewed changes

jimi567 requested changes Oct 2, 2024

View reviewed changes

J-I-H-O reviewed Oct 4, 2024

View reviewed changes

ehtjsv2 previously approved these changes Oct 4, 2024

View reviewed changes

J-I-H-O requested changes Oct 6, 2024

View reviewed changes

refactor: 기존 메서드를 StringUtils.getFilenameExtension()로 대체

d8cd996

takoyakimchi dismissed ehtjsv2’s stale review via d8cd996 October 6, 2024 08:56

ehtjsv2 approved these changes Oct 8, 2024

View reviewed changes

J-I-H-O approved these changes Oct 10, 2024

View reviewed changes

jimi567 approved these changes Oct 10, 2024

View reviewed changes

takoyakimchi merged commit f3c00c3 into develop Oct 10, 2024
3 checks passed

takoyakimchi deleted the feature/#216-image branch October 10, 2024 08:00

takoyakimchi added a commit that referenced this pull request Oct 23, 2024

[BE] fix: .jpg 이외의 이미지도 업로드할 수 있게 수정 (#605)

b141c79

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BE] fix: .jpg 이외의 이미지도 업로드할 수 있게 수정 #605

[BE] fix: .jpg 이외의 이미지도 업로드할 수 있게 수정 #605

takoyakimchi commented Oct 2, 2024 •

edited

Loading

github-actions bot commented Oct 2, 2024 •

edited

Loading

ehtjsv2 left a comment

ehtjsv2 Oct 2, 2024

takoyakimchi Oct 2, 2024 •

edited

Loading

jimi567 Oct 2, 2024

takoyakimchi Oct 2, 2024

J-I-H-O Oct 6, 2024

J-I-H-O Oct 6, 2024

takoyakimchi Oct 6, 2024 •

edited

Loading

jimi567 left a comment

jimi567 Oct 2, 2024

J-I-H-O Oct 2, 2024

takoyakimchi Oct 4, 2024

ehtjsv2 left a comment

J-I-H-O Oct 6, 2024

J-I-H-O Oct 6, 2024

ehtjsv2 left a comment

[BE] fix: .jpg 이외의 이미지도 업로드할 수 있게 수정 #605

[BE] fix: .jpg 이외의 이미지도 업로드할 수 있게 수정 #605

Conversation

takoyakimchi commented Oct 2, 2024 • edited Loading

이슈

개발 사항

전달 사항 (없으면 삭제해 주세요)

github-actions bot commented Oct 2, 2024 • edited Loading

Test Results

ehtjsv2 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

takoyakimchi Oct 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

takoyakimchi Oct 6, 2024 • edited Loading

Choose a reason for hiding this comment

jimi567 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ehtjsv2 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ehtjsv2 left a comment

Choose a reason for hiding this comment

takoyakimchi commented Oct 2, 2024 •

edited

Loading

github-actions bot commented Oct 2, 2024 •

edited

Loading

takoyakimchi Oct 2, 2024 •

edited

Loading

takoyakimchi Oct 6, 2024 •

edited

Loading