[teo.kkim 김태홍] WAS 미션 추가 PR입니다. - #71
Conversation
- 헤더의 키-밸류에서 밸류는 세미콜론이나 콤마로 구분되어 여러 값을 가질 수 있다.
- 미리 구현된 프론트에서 *.html 형태로 요청이 들어오는 경우가 있다.
karian7
left a comment
karian7
left a comment
There was a problem hiding this comment.
안녕하세요. 테오. 발령난지 한달가까이 되어가는데 잘 지내고 계신가요?
지난 6일날 리뷰요청 주셔서 기뻤는데 주중이 되니 완전이 잊어먹어서 놓치고 있었어요. 죄송합니다. 🙏
발령된 팀에서 여러가지 배우느라 바쁘실텐데, 이렇게 기존의 배움을 이어나가는 부분이 너무 좋습니다. 👏👏👏
이제라도 피드백 드릴 수 있어서 좋고요, 앞으로도 언제든 궁금하신 점 있을때 도와드릴 수 있으면 좋겠습니다.
PR 내용에 대해서 피드백 남겼습니다. 보시고 다시 요청 주세요~
| return parameters; | ||
| } | ||
|
|
||
| private static class Pair { |
| String[] tokens = input.split(DELIM_AMPERSAND); | ||
| for (String token : tokens) { | ||
| Pair pair = Pair.from(token); | ||
| parameters.put(pair.key, pair.value); |
There was a problem hiding this comment.
guava 라이브러리를 사용할 수 있다면 이렇게도 가능해요.
Map<String, String> parameters = Splitter.on("&").withKeyValueSeparator("=").split(input);| Map<String, String> parameters = KeyValueTokenizer.of(input); | ||
| return new User(parameters); | ||
| public User(Map<String, String> params) { | ||
| this(params.get("userId"), params.get("password"), params.get("name"), params.get("email")); |
| } | ||
|
|
||
| public String getHeader(String key) { | ||
| public List<String> getHeader(String key) { |
There was a problem hiding this comment.
쿠키 헤더에는 여러 정보가 담길 수 있다. 이를 고려해보는 것이 좋겠다.
링크 주신 rfc 문서를 통해 기존 Map<String, String> 구조를 Map<String, List> 의 멀티밸류맵으로 변경했습니다. 쿠키만 따로 일급 컬렉션으로 만들지도 고민했지만, 다른 헤더도 단순 1:1 매핑을 했을때 문제가 생길 수 있다고 느껴서 헤더 전체를 변경하게 되었습니다.
지난번 코맨트에는 쿠키 헤더 스펙에 관한것이었고 http header 에는 value 가 어떤 형태인지 기술되어 있지 않습니다. 다시말해 http header 스펙안에서 쿠키헤더 스펙이 존재하는것이죠.
지금 수정은 http header 스펙을 쿠키 스펙으로 구현해 놓은것이라서 좋지 않아 보여요.
assertThat(headers.getHeader("cookie")).isEqualTo("aaa=bbb; ccc=ddd");
assertThat(cookies.getValue("aaa")).isEqualTo("bbb");이렇게 구현되어 있어야 양쪽의 스펙을 모두 맞춘 결과물이 나올 수 있어 보입니다.
| assertThat(user.getUserId()).isEqualTo("javajigi"); | ||
| assertThat(user.getPassword()).isEqualTo("password"); | ||
| assertThat(user.getName()).isEqualTo("자바지기"); | ||
| assertThat(user.getEmail()).isEqualTo("javajigi%40slipp.net"); |
There was a problem hiding this comment.
현재 @과 같은 특수기호가 %40으로 나타납니다. 확인해보니 이미 요청을 받아서 파라미터를 파싱하고 난 결과값부터 %40으로 나오더라구요. 아마 HttpRequest.from() 메서드에서 인풋스트림을 변환할 때 함께 바뀌는 것 같습니다. UTF-8 인자를 빼도 동일한 증상이 나타나는데 원인이 궁금합니다.
POST 메소드의 body 의 content type은 application/x-www-form-urlencoded 이며 아래와 같이 처리됩니다. 참고
application/x-www-form-urlencoded: &으로 분리되고, "=" 기호로 값과 키를 연결하는 key-value tuple로 인코딩되는 값입니다. 영어 알파벳이 아닌 문자들은 percent encoded 으로 인코딩됩니다. 따라서, 이 content type은 바이너리 데이터에 사용하기에는 적절치 않습니다. (바이너리 데이터에는
multipart/form-data를 사용해 주세요.)
브라우저가 위 스펙에 맞게 body 를 인코딩 해서 전달한 것이고 따라서 서버는 스펙에 맞게 디코딩 해야해요.
|
|
||
| boolean isLogin(String header) { | ||
| return header.equals(LOGINED_TRUE); | ||
| private final Logger logger = LoggerFactory.getLogger(ListUserController.class.getName()); |
There was a problem hiding this comment.
| private final Logger logger = LoggerFactory.getLogger(ListUserController.class.getName()); | |
| private final Logger logger = LoggerFactory.getLogger(getClass()); |
로거를 스테틱으로 만들지 않는다면 이 방법이 더 좋아요.
| httpResponse.forward(url, body); | ||
| } catch (IOException e) { | ||
| e.printStackTrace(); | ||
| logger.info(e.getMessage()); |
There was a problem hiding this comment.
| logger.info(e.getMessage()); | |
| logger.error(e.getMessage(), e); |
특별한 이유가 없다면 로거 레벨은 error로 하고 예외객체를 전달하여 스텍트레이스가 남도록 합니다.
| boolean isLoginSuccess = user.validatePassword(httpRequest.getParameter(KEY_PASSWORD)); | ||
| if (isLoginSuccess) { | ||
| httpResponse.addHeader(KEY_SET_COOKIE, VALUE_LOGINED_TRUE); | ||
| httpResponse.addHeader(KEY_SET_COOKIE, cookieWithExpireTime()); |
There was a problem hiding this comment.
의도하신게 10초동안 쿠키가 유효하게 하려는 걸까요? 그렇다면..
| httpResponse.addHeader(KEY_SET_COOKIE, cookieWithExpireTime()); | |
| httpResponse.addHeader(KEY_SET_COOKIE, "logined=true; Path=/; max-age=10"); |
이렇게 해도 됩니다. 다만, 10초 후엔 로그인이 풀리게 되는데 의도가 이게 맞는지 궁금하네요 😀
일반적으로는 시간 설정을 안하고(=브라우저가 닫히면 로그아웃) 로그아웃 버튼을 눌렀을 때 수명을 조정하여 브라우저가 삭제하도록 만듭니다.
httpResponse.addHeader(KEY_SET_COOKIE, "logined=true; Path=/; max-age=0");3 participants
안녕하세요! 지난 리뷰를 반영한 PR입니다. kit의 리뷰 기회는 놓칠 수 없습니다.. ✍️🔥
앞서 짚어주신 코멘트에 대한 고민을 담아보았습니다.
쿠키 헤더에는 여러 정보가 담길 수 있다. 이를 고려해보는 것이 좋겠다.
링크 주신 rfc 문서를 통해 기존
Map<String, String>구조를Map<String, List<String>>의 멀티밸류맵으로 변경했습니다. 쿠키만 따로 일급 컬렉션으로 만들지도 고민했지만, 다른 헤더도 단순 1:1 매핑을 했을때 문제가 생길 수 있다고 느껴서 헤더 전체를 변경하게 되었습니다./user/list.html에서 의도한 동작이 되지 않는다.힌트: 응답헤더말씀해주신대로 응답 헤더 문제가 존재함을 확인했습니다. 로그인 쿠키가 false, true 둘 다 들어가서 결과적으로 로그인이 항상 실패하는 형태였어요. 결국에는 앞서 쿠키 헤더 관련 문제와도 엮여 있었습니다. 이 부분은
logined=falselogined=true로 로그인 쿠키를 관리하는 기존 구조에 문제가 있다고 판단했습니다.logined=false라는 상태를 없애고,logined=true에 쿠키 유효시간을 추가하는 식으로 구현했습니다.패스워드나 아이디 하나만 채우면 이상하게 작동하는 문제는 토크나이저에 원인이 있다
이 문제도 말씀해주신대로
logger를 사용해 찾았습니다. 입력하지 않으면 당연히 빈 문자열로 파싱될 것이라고 안일하게 생각했습니다 😭 이 부분은 키와 밸류 쌍을 표현하는Pair클래스를 도입해서 고쳐보았습니다.값이 없음을 No Key로 표현하는 것은 문제가 있다
이것은 앞서 헤더의 밸류가
List로 바뀐 점과, 최근 읽은 이펙티브 자바의 null 보다 빈 리스트를 반환하라는 조언을 참고하여, 값이 없는 상황에서Collections.emptyList()를 반환하도록 고쳤습니다.의문점 :
현재
@과 같은 특수기호가%40으로 나타납니다. 확인해보니 이미 요청을 받아서 파라미터를 파싱하고 난 결과값부터%40으로 나오더라구요. 아마HttpRequest.from()메서드에서 인풋스트림을 변환할 때 함께 바뀌는 것 같습니다. UTF-8 인자를 빼도 동일한 증상이 나타나는데 원인이 궁금합니다.매번 많이 배우고 있습니다. 감사합니다 😃