Nexters / BandalArt-Android

부담 없는 만다라트 계획표로 당신의 목표를 더욱 선명하게, 반다라트 Android
https://play.google.com/store/apps/details?id=com.nexters.bandalart
23 stars 0 forks source link

Retrofit Provider 분리 #144

Closed easyhooon closed 1 year ago

easyhooon commented 1 year ago
easyhooon commented 1 year ago

Bitrise가 갑자기 왜 동작하지 ㄷ bitrise fail뜬거는 무시해도 될듯.. 확인해봐야겠다

easyhooon commented 1 year ago

그러면 왜 이렇게 되는 걸까

addHeader를 통해 header 를 추가 할때(key, value의 형식), value 가 유효한 값이 아닌 경우(nullOrEmpty) header 에 추가되지 않는 것 같아. (addHeader 함수를 타고 들어가면 아래 코드를 확인할 수 있음)

image

하지만 key를 check하는 함수와는 다르게 (headersCheckName) 값이 유효하지 않은 경우 IllegalArgumentException 을 던져버리지는 않는 듯 함.. 원래도 동작하는덴 이상은 없었으나, 코드 단에서 명시적으로 Header가 존재하는 Retrofit Provider 와 Header가 존재하지 않는 Retrofit Provider 를 구분하는게 그 결과를 예상할 수 있기에 더 나은 방법일 듯 함

likppi10 commented 1 year ago

지훈이형이 띄워준 캡쳐에는

addHeader를 통해 header 를 추가 할때(key, value의 형식), value 가 유효한 값이 아닌 경우(nullOrEmpty) header 에 추가되지 않는 것 같아.

이와 같은 내용은 확인되지 않는걸,,? 그냥 isSeneitiveHeader 일때만 ""로 하겠다는 의미만 있는 것 아닌가?

image

민감한 정보는 이런걸 의미한다는 구만,,

image

어찌됐든 우리는 현재 유제 토큰의 첫 값(값이 없을 때)을 ""으로 보내주고 있으니 null이 아니기 때문에 헤더에는 담겨가고 있는것 같아

image

null이 들어오면 헤더를 추가하지 않는게 아니라 오류가 뜨지 않을까? 함수가 null 자체를 허용하지 않는걸..?

easyhooon commented 1 year ago

함수가 null 자체를 허용하지 않는걸..?

ㅇㅎ 잘못 말했네 내가 nullOrEmpty 가 아니라 Emtpy 일때만 인게 맞겠구만

해당 이미지를 보면 확실히 처음 다운로드가 됐을 때는, 헤더에 아무것도 담기지 않음 그리고 연이어 호출되는 연결에 헤더가 담김

헤더에 추가되지 않는다는 건 이걸 통해 추론해본거였어 지금은 이미 merge 되어가지고 저때 코드 버전으로 돌려보진 않아서

likppi10 commented 1 year ago

아무튼 레트로핏은 생각보다 예민한 네이밍을 지양하는 것 같기도 하고, 이번 이슈는 그냥 로컬에서 찾으려는 값이 없으면 ""이다 에서 어쩌다 스무스하게 넘어가게된 일이 된거네 거럼

easyhooon commented 1 year ago

서버에서 그렇게 header 가 넘어갈 경우 어떤식으로 핸들링 하는지 물어봐도 좋을 듯하네 헤더에 value 값이 "" 로 왔을때 이게 뭐야 이러고 쳐내버리는지, 아니면 유효하지 않은 값(빈값)이면 이걸 아예 무시해버리는지

image 일단 위에 올린 이 함수들이 클라이언트 단에서 validation을 검사하는 함수들인데 value 로 "" 같은 값이 들어가면 for 문 자체를 바로 빠져나와버리게 되어서 함수가 검사라는 역할을 못해주긴 하는것 같아 for 문 내부로 진입을 해야 illegalArgumentException을 던질 수 있는데 뭐 반환 값 같은 것도 없구 ㅇㅇ

easyhooon commented 1 year ago

아무튼 레트로핏은 생각보다 예민한 네이밍을 지양하는 것 같기도 하고,

나도 sensitive 한 목록 중 Authorization 밖에 써보진 않았는데 Jwt 토큰을 통해 통신할 땐 쓰이는 헤더 네임이거든

("Authorization", "Bearer $AccessToken") 이런식으로 ㅇㅇ 아마 나머지는 jwt 토큰을 통한 방식이 아니라, 세션, 쿠키등을 이용한 통신이긴 할거야 많이 쓰일거구 다만 이런 헤더들을 sensitive 하게 음.. 좀 특별하게 다루는 듯

-> 아 다시 보니깐 Exception에 메세지를 로그에 뿌릴 때 검사하는거네 그렇지 아무래도 jwt 토큰이든, 세션이든 쿠키이든 다 민감한 정보이니까 Exception message 를 뿌리기 전에 민감한 정보라고 판단되면 감춰버리는 듯함 "" 로, 로그를 확인해서 토큰을 갈취할 수도 있는 것이니까, 얘네가 아니면 별로 민감한 정보라고 판단하지 않는 경우에만 로그에 $value 로 그냥 header 의 value 를 공개 해버리는 식인듯!

likppi10 commented 1 year ago

아 그러면 헤더에 담기는 현상 자체는

우리는 현재 유제 토큰의 첫 값(값이 없을 때)을 ""으로 보내주고 있으니 null이 아니기 때문에 헤더에는 담겨가고 있는것 같아

로 설명되는거고

저 캡쳐들은 예외로그 띄울 때 값을 보여주지 않기 위해 감춰둔거구나

easyhooon commented 1 year ago

저 캡쳐들은 예외로그 띄울 때 값을 보여주지 않기 위해 감춰둔거구나

https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/require.html ㅇㅇ kotlin의 저 require 문법이 괄호안에 있는 조건을 만족하지 않으면 illegalArgumentException을 던지는 함수인데 우리 ktlint, detekt 검사할때도 illegalArgumentException 를 직접 던지지 말고 kotlin 문법에서 권장하는 require 를 쓰라고 검사해주고 있음 ㅇㅇ

likppi10 commented 1 year ago

아주 멋지군.. 👍