[톰캣 구현하기 1, 2단계] 안나 (김민겸) 미션 제출합니다. #571
Merged
Conversation
This file contains 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
jinchiim
requested changes
Sep 7, 2024
There was a problem hiding this comment.
안녕하세요 안나! 폴라입니다!
요구사항을 잘 지킨채로 열심히 구현해주셨네요!
전반적으로 메서드 이름이나 변수 이름들을 정말 잘 지어주셔서 리팩토링을 챙기지 못하셨음에도 이해가 빠르게 되었습니다! 👍🏻
몇가지 부분들도 보며 저도 배워가는 시간이었네요.
제가 리뷰를 잘못 달았다거나, 의도가 전달되지 않았다고 생각하는 부분들은 편하게 DM 주셔도 괜찮습니다!
천천히 답변 달아주세요!
+) 몇가지 의견들은 3단계 리팩토링에서 할 수 있는 부분들이기 때문에 이번 리뷰에서 반영할지, 다음 리뷰에서 반영할지 선택하여 말씀해주셔도 좋습니다! 😄
tomcat/src/main/java/org/apache/coyote/http11/request/HttpRequest.java
Outdated
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/request/RequestHandler.java
Outdated
Show resolved
Hide resolved
Comment on lines
61
to
62
| public static void redirect(String location, OutputStream outputStream) { | ||
| try { |
There was a problem hiding this comment.
오 redirect 라는 이름 너무 좋네요 굉장히 직관적입니다... 👍🏻
There was a problem hiding this comment.
그렇지만 역따봉은 조금 슬퍼서 resolve를 못하겠네요ㅜ
tomcat/src/main/java/org/apache/coyote/http11/request/RequestHandler.java
Outdated
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/request/RequestHandler.java
Outdated
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/response/ResponseHandler.java
Outdated
Show resolved
Hide resolved
|
폴라 !!! 꼼꼼하게 리뷰 남겨주어서 넘넘 감사합니다 😍 주중에는 시간이 많이 없어서 미션에 대해 깊이 파고들지 못했었는데, 폴라 리뷰 덕에 HTTP 캐싱이나 객체 분리 등에 대해서 한 번더 돌아보고 정리하는 계기가 되었어요. |
jinchiim
approved these changes
Sep 8, 2024
Comment on lines
+36
to
+37
| } | ||
| return null; |
There was a problem hiding this comment.
헤더는 조작될 수도 있지 않을까요??
예를 들어 예상한 값보다 크게 작성한다면 헤더가 아직 전송중이라고 파악할 수도 있을 것 같습니다!
저도 몰랐는데 이를 찾아보니, Slow Http Dos 공격이라고도 부른다더라고요! 🫨
이 사실을 알고 저도 예외처리를 빡세게 해보려고 하는데 같이 시작하는 것은 어떨까요? 😋
tomcat/src/main/java/org/apache/coyote/http11/request/RequestHandler.java
Outdated
Show resolved
Hide resolved
Comment on lines
61
to
62
| public static void redirect(String location, OutputStream outputStream) { | ||
| try { |
There was a problem hiding this comment.
그렇지만 역따봉은 조금 슬퍼서 resolve를 못하겠네요ㅜ
tomcat/src/main/java/org/apache/coyote/http11/request/RequestHandler.java
Outdated
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/request/RequestHandler.java
Outdated
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/response/ResponseHandler.java
Outdated
Show resolved
Hide resolved
Mingyum-Kim
added a commit
to Mingyum-Kim/java-http
that referenced
this pull request
Sep 12, 2024
* test: 학습 테스트 진행 * feat: `/index.html`에 접근했을 때 static/index.html 파일이 보여지도록 구현 * feat: 자원에 따라 다른 Conent-Type으로 응답을 보내도록 수정 * fix: remove implementation logback-classic on gradle (woowacourse#501) * fix: 공백 처리 때문에 index.html이 불러와지지 않는 현상 해결 * feat: 로그인 요청 시 쿼리 파라미터를 파싱하는 기능 구현 * fix: add threads min-spare configuration on properties (woowacourse#502) * feat: 로그인에 성공하거나 실패한 경우 다른 페이지로 리다이렉트하는 * feat: POST 요청으로 회원 가입 시 요청 본문에서 정보를 가져와 사용자 저장하는 기능 구현 * feat: 로그인 시 POST 요청을 보내도록 수정 * feat: 로그인 시 세션 아이디를 쿠키에 등록하는 기능 구현 * feat: 쿠키에 JSESSIONID가 없을 때 Set-Cookie를 발급하는 기능 구현 * feat: 이미 로그인한 회원의 경우 로그인 페이지에 접근 불가하도록 구현 * refactor: 쿠키가 없을 때 일반 페이지에 접근하면 세션 아이디를 주지 않음 * feat: HTTP 활용 학습 테스트 진행 * refactor: 뭉친 코드를 클래스 분리하여 리팩토링 * fix: JSESSIONID를 추가하지 않아 로그인되지 않는 문제 해결 * fix: 세션 아이디가 달라서 로그인 시 로그인 페이지가 접속되는 현상 해결 * refactor: 기존 코드 주석 제거 * style: 주석 제거 및 컨벤션 맞추기 * refactor: 미구현된 파일 제거 * refactor: 필터를 사용하여 ETag를 지정하는 것으로 변경 * refactor: 인터셉터에서 CacheControl을 적용할 수 있도록 수정 * refactor: Content-Length 헤더 이름을 상수화 * refactor: 정적 파일을 한 번에 처리하도록 리팩토링 * fix: Json 파일을 보내는 경우를 고려하여 MediaType 수정 * refactor: 로그인 시 쿠키 세션을 꺼내오는 코드 가독성 개선 * refactor: 요청한 사용자 정보가 없는 경우 401 페이지를 보여주도록 수정 * refactor: 쿠키의 키값을 상수화하여 리팩토링 --------- Co-authored-by: Gyeongho Yang <[email protected]>
jinchiim
pushed a commit
that referenced
this pull request
Sep 13, 2024
* fix: remove implementation logback-classic on gradle (#501) * fix: add threads min-spare configuration on properties (#502) * [톰캣 구현하기 1, 2단계] 안나 (김민겸) 미션 제출합니다. (#571) * test: 학습 테스트 진행 * feat: `/index.html`에 접근했을 때 static/index.html 파일이 보여지도록 구현 * feat: 자원에 따라 다른 Conent-Type으로 응답을 보내도록 수정 * fix: remove implementation logback-classic on gradle (#501) * fix: 공백 처리 때문에 index.html이 불러와지지 않는 현상 해결 * feat: 로그인 요청 시 쿼리 파라미터를 파싱하는 기능 구현 * fix: add threads min-spare configuration on properties (#502) * feat: 로그인에 성공하거나 실패한 경우 다른 페이지로 리다이렉트하는 * feat: POST 요청으로 회원 가입 시 요청 본문에서 정보를 가져와 사용자 저장하는 기능 구현 * feat: 로그인 시 POST 요청을 보내도록 수정 * feat: 로그인 시 세션 아이디를 쿠키에 등록하는 기능 구현 * feat: 쿠키에 JSESSIONID가 없을 때 Set-Cookie를 발급하는 기능 구현 * feat: 이미 로그인한 회원의 경우 로그인 페이지에 접근 불가하도록 구현 * refactor: 쿠키가 없을 때 일반 페이지에 접근하면 세션 아이디를 주지 않음 * feat: HTTP 활용 학습 테스트 진행 * refactor: 뭉친 코드를 클래스 분리하여 리팩토링 * fix: JSESSIONID를 추가하지 않아 로그인되지 않는 문제 해결 * fix: 세션 아이디가 달라서 로그인 시 로그인 페이지가 접속되는 현상 해결 * refactor: 기존 코드 주석 제거 * style: 주석 제거 및 컨벤션 맞추기 * refactor: 미구현된 파일 제거 * refactor: 필터를 사용하여 ETag를 지정하는 것으로 변경 * refactor: 인터셉터에서 CacheControl을 적용할 수 있도록 수정 * refactor: Content-Length 헤더 이름을 상수화 * refactor: 정적 파일을 한 번에 처리하도록 리팩토링 * fix: Json 파일을 보내는 경우를 고려하여 MediaType 수정 * refactor: 로그인 시 쿠키 세션을 꺼내오는 코드 가독성 개선 * refactor: 요청한 사용자 정보가 없는 경우 401 페이지를 보여주도록 수정 * refactor: 쿠키의 키값을 상수화하여 리팩토링 --------- Co-authored-by: Gyeongho Yang <[email protected]> * refactor: Method, Body, Path 를 VO로 분리 * refactor: HttpResponse와 하위 요소들을 VO로 분리 * refactor: HttpResponse를 활용해 응답을 보내는 코드 리팩토링 * refactor: Content-Type을 객체로 생성하여 생성 로직 위임 * refactor: 응답 본문을 반환하는 부분을 객체로 대체 * refactor: 생성자와 요청 HTTP 생성 용 정적 팩토리 메서드 분리 * refactor: initialLine을 requestLine으로 변수명 변경 * refactor: 서블릿 컨테이너를 사용해서 요청을 처리하도록 수정 * refactor: 동적 요청에 대해 분기하는 HandlerMapping 구현 * refactor: Servlet이 적절한 컨트롤러를 꺼내어 처리하도록 구현 * refactor: HttpResponse 빈 객체 생성 후 값을 대입할 수 있도록 수정 * refactor: 응답을 출력하는 역할을 하는 클래스 분리 * feat: 로그인을 담당하는 컨트롤러 구현 * feat: 회원 저장을 담당하는 Controller 구현 * feat: 정적 파일 처리를 담당하는 Controller 구현 * refactor: 분기 처리를 구현 컨트롤러에게 위임하여 로직 개선 * refactor: Body에서 속성을 파싱할 수 있도록 Properties 클래스 분리 * fix: 로그인 페이지가 출력되지 않는 버그 해결 * fix: 세션 정보가 조회되지 않는 현상 해결 * fix: 실패하는 테스트 케이스 수정 * feat: Executors로 Thread Pool 적용 * refactor: 기존의 Controller로 명명한 클래스 이름을 Servlet으로 수정 * test: 로그인 및 회원가입 서블릿 테스트 구현 * refactor: 필요한 설정 값을 상수화하도록 수정 * style: 클래스 상단 개행 컨벤션 맞춤 * refactor: 정적 파일 처리 시 . 을 붙여 확장자 판별 * refactor: 모든 헤더 키를 상수화 * refactor: 정적 메서드의 이름을 직관적으로 변경 * refactor: HTTP 파싱을 위한 Delimter 를 전부 상수화 * refactor: 세션 값을 추출하는 메서드를 분리 * refactor: RequestLine과 ResponseLine으로 객체를 추상화 * refactor: 헤더 키를 상수화하여 생성하도록 수정 * fix: 인덱스 페이지가 연결되지 않는 현상 해결 * refactor: GET, POST 이외의 메소드 요청이 온다면 405를 반환 * refactor: 프로퍼티를 HttpRequest에서 바로 꺼내오도록 설정 * refactor: 로그인이 실패한 경우 401 페이지를 응답 * refactor: RequestLine에서 한번에 처리하도록 수정 * refactor: 이미 존재하는 유저가 아니라면 저장하도록 수정 * fix: 불일치 메서드명 수정 * fix: 요청과 응답이 동작하지 않는 문제 해결 * refactor: ContentLength가 0 이하인 경우 예외처리 * refactor: 정적 메서드의 이름을 수정 * fix: 패스워드가 조회되지 않는 문제 해결 * fix: 루트경로 입력시 Hello, World가 보여지도록 수정 * test: 실패하는 테스트 케이스 수정 * fix: 잘못 올라간 파일 수정 --------- Co-authored-by: Gyeongho Yang <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
안녕하세요 폴라 🍇
안나입니다!
최종 코테 끝나고 폴라랑 리뷰 주고받았었는데, 이렇게 다시 또 리뷰를 받게 되네요 😎
시간이 많이 없어서 리팩토링을 챙기지 못했어요 😅 미리 죄송하다는 말씀 드립니다.
따가운 피드백 부탁드려요 ! 감사합니다 😀