[톰캣 구현하기 - 1, 2단계] 테오(최우성) 미션 제출합니다. #304
Conversation
tomcat/src/main/java/org/apache/coyote/http11/handler/RegisterHandler.java
Outdated
Show resolved
Hide resolved
| if (reader.ready()) { | ||
| char[] buffer = new char[1024]; | ||
| reader.read(buffer); | ||
| body.addAll(Arrays.asList(new String(buffer).trim() | ||
| .split("&"))); | ||
| } |
There was a problem hiding this comment.
body로 들어오는 값이 길어진다면 잘릴 위험이 있을 것 같아요 !
또한 body값으로 &가 delimiter가 아닌 값이 들어올 수도 있지 않을까요 !?
There was a problem hiding this comment.
말씀해주셔서 Request Body에 어떤 값들이 들어올 수 있는지 조사를 조금 해보았는데요!
현재는 HTML 파일의 form 태그를 통해 로그인 정보를 보내는 방식이다보니
기본으로 Content-Type을 application/x-www-form-urlencoded 로 사용합니다.
이 경우 데이터가 key=value&key=value 형식으로 들어오게 되고, 그렇기 때문에 위의 코드가 정상적으로 작동하는 것 같습니다.
토리가 말씀주신대로 Content-Type에 따라 여러 Delimiter가 들어올 수 있더라구요.
Postman으로 form-data를 작성해 요청해보기도 했는데, 이 경우에는 multipart/form-data를 사용하고 delimiter를 클라이언트가 정의하기 때문에 파싱 자체가 불가능했었습니다 🥲
현재는 다양한 Content-Type을 수용할 필요는 없으나 추가적으로 JSON 정도까지만 파싱이 가능하게 구현해보려고 하는데, 어떻게 생각하시는지 궁금합니다!
There was a problem hiding this comment.
body buffer 크기 부분은 Content-Length 를 사용하는 쪽으로 수정해보도록 하겠습니다!
There was a problem hiding this comment.
얘기나눴던 부분인데 다시 정리해서 리뷰 남기자면 !
제가 의도한 방향은 "&"로 파싱하는 부분을 한 단계 밖이나, 실제로 body값에 대한 파싱이 필요한 부분에서 실행하는게 어떨까 였습니다 !
외부에서 하면 조금 더 content-type에 따라 분기를 나눠서 실행하기도 편하고 로직이 추가되기도 편할 것 같았습니다 ㅎㅎ
말씀해주신대로 현재는 다양한 Content-Type을 수용할 필요가 없으니 따로 파싱을 추가하지 않아도 괜찮을 것 같아요 👍
tomcat/src/main/java/org/apache/coyote/http11/message/Headers.java
Outdated
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/message/request/RequestBody.java
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/handler/Handlers.java
Outdated
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/message/HttpStatus.java
Outdated
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/handler/IndexHandler.java
Outdated
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/handler/LoginHandler.java
Outdated
Show resolved
Hide resolved
tomcat/src/main/java/org/apache/coyote/http11/handler/LoginHandler.java
Outdated
Show resolved
Hide resolved
| @@ -48,11 +48,11 @@ class OutputStream_학습_테스트 { | |||
| void OutputStream은_데이터를_바이트로_처리한다() throws IOException { | |||
| final byte[] bytes = {110, 101, 120, 116, 115, 116, 101, 112}; | |||
There was a problem hiding this comment.
해당 학습 테스트는 고쳐지지 않은 것 같은데 시간 될 때 해보시면 좋을 거 같아요 ˙ᵕ˙ 👍🐤
|
Kudos, SonarCloud Quality Gate passed!
|
| private static RequestBody extractBody(BufferedReader reader, int contentLength) throws IOException { | ||
| List<String> body = new ArrayList<>(); | ||
| if (reader.ready()) { | ||
| char[] buffer = new char[contentLength]; |
| if (reader.ready()) { | ||
| char[] buffer = new char[1024]; | ||
| reader.read(buffer); | ||
| body.addAll(Arrays.asList(new String(buffer).trim() | ||
| .split("&"))); | ||
| } |
There was a problem hiding this comment.
얘기나눴던 부분인데 다시 정리해서 리뷰 남기자면 !
제가 의도한 방향은 "&"로 파싱하는 부분을 한 단계 밖이나, 실제로 body값에 대한 파싱이 필요한 부분에서 실행하는게 어떨까 였습니다 !
외부에서 하면 조금 더 content-type에 따라 분기를 나눠서 실행하기도 편하고 로직이 추가되기도 편할 것 같았습니다 ㅎㅎ
말씀해주신대로 현재는 다양한 Content-Type을 수용할 필요가 없으니 따로 파싱을 추가하지 않아도 괜찮을 것 같아요 👍
tomcat/src/main/java/org/apache/coyote/http11/message/Headers.java
Outdated
Show resolved
Hide resolved
| public static Headers fromMap(Map<String, String> mappings) { | ||
| return new Headers(mappings); | ||
| } | ||
|
|
||
| public static Headers fromLines(List<String> lines) { |
| CONTENT_TYPE("Content-Type"), | ||
| CONTENT_LENGTH("Content-Length"), | ||
| SET_COOKIE("Set-Cookie"), | ||
| LOCATION("Location"); |
|
|
||
| verify(outputStream, atLeastOnce()).close(); | ||
| verify(outputStream, atLeastOnce()).close(); | ||
| } |
There was a problem hiding this comment.
try 안의 outputStream을 밖에서 선언해준 뒤 변수를 사용하면 확실하게 검증이 될 거 같네요 !!
안녕하세요 토리! 반갑습니다~
1, 2단계 구현 마치고 리뷰 요청드립니다!
아래는 큰 흐름을 도식화한 그림인데, 리뷰하실 때 참고하시면 좋을 것 같아요 🙇♂️