이전 글(https://velog.io/@siwol406/Sql-Injection%EC%9D%84-%EB%A7%89%EC%95%84%EB%B3%B4%EC%9E%90)에서 sql injection을 막기 위해 대량의 코드를 수정했다고 글을 썼었다. 그 이후로 어떤일이 발생했고 어떤 식으로 일을 헤쳐나아갔는지 알아보며 다시 실수 하지 않기 위해 기록하자
문제 발생
동적 바인딩때문에 발생한 문제
SQL injection을 막기 위해 동적 바인딩을 사용했다. 그러다 보니 sql 구문이 sprintf를 이용하여 sql 구문을 만드는 것에 비해 제약이 많았고 조건이 많이 추가되었다. 바인딩하는 값의 자료 타입으로 인해 구문이 제대로 만들어지지 않던가(ex [1,2,3,4] 처럼 array오는 값들을 동적 바인딩 하려면 각 값을 다 동적바인딩 시켜야함 [1,2,3,4] 배열 전체를 값에 넣을 수 없음 1,2,3,4가 string으로 온다고 하더라도 값전체를 바인딩 시켜버리면 구문에러는 발생하지 않으나 값을 제대로 인식 못하는 경우 발생), 데이터 컬럼과 바인딩 시 key값의 불일치 등의 여러가지 요인들로 인해 잔실수 등이 발생했다.
escape문 처리
%, _ 등 sql문의 와일드카드로 인식되는 값 들을 와일드카드로 인식하지 않고 특수문자 그대로 인식하기 위해서는 동적바인딩 만으로는 불가능 하다. 그래서 escape를 활용해서 값들을 인식 시키는데 escape문은 Like 구문에서만 사용가능하나 조건 처리를 제대로 못해서 like 구문이 아닌 곳에도 escape가 추가되어 동작 에러가 발생했다.
escape 사용법 => SELECT * FROM table_name WHERE column_name LIKE '%\%%' ESCAPE '\'; 이런식으로
함수의 문제
여러 페이지를 동적 바인딩을 위해서 수정해야하다 보니 이런 검색 필터에서 사용되는 값들을 필터링 해주는 것들을 함수로 만들어서 preprocessing 전처리 해주는 함수를 만들어 공통 함수를 만들어 필터링 처리를 해주면 코드의 길이도 줄어들고 preprocessing 클래스만 수정하면 되니까 유지 보수가 쉬울 것이라고 생각했다.
- 함수 만들기 : 단 함수 두개로 회사 제품에서 모든 검색기능의 필터링을 처리하고자 하였다. 그러다 보니 변수의 수가 6가지 정도 되었고 함수의 복잡도가 굉장히 커졌다. 예를 들어 key값과 데이터 컬럼의 이름 차이 여부, like 사용 여부, $value의 자료 타입 등의 여부 확인 및 escape 구문 사용 여부 등을 확인하고 처리하는 함수를 만들다 보니 함수의 복잡도가 매우 커졌다.
예를 들어 preprocessingValue() 함수는 $filter['name']의 값이 들어오면 해당 name을 이용해서 where 구문을 만들어주는 함수다. name like :name, $args = ['name': 'chulsoo'] 이렇게 동적 바인딩 하기 위한 where구문과 바인딩 값들을 만들어 넣어준다.
- 거기다 위의 복잡한 함수를 이용한 또 모든 필터를 처리해서 where문을 만드는 함수를 다시 제작했다. 예를들어 $filter = ['name' : 'chulsoo', 'age':'20']처럼 $filter의 값이 여러개가 나올 때 해당 값들을 위의 함수를 이용해서 다 where구문을 만드는 함수를 다시 제작했다.
=> 여기서 문제가 발생했다. $filter에 들어오는 값과 데이터 컬럼값 모든 것들이 내가 예상하는 범위 밖의 크기와 다양성이엇다. 같은 name이란 변수를 사용하지만 특정 서비스에서는 name like %name%을 쓰기도하고 다른 곳에서는 special_name like %name% 등 특정 조건에 where문이 다 다르게 나오는 경우가 많이 존재했다. 그러다 보니 내가 사용하는 함수르 제대로 활용하기도 어려웠고 이것저것 조건문을 붙여가며 함수를 만들다 보니 이곳에서 문제가 터졌다.
범위 예상 못함
filter를 처리하는 양을 제대로 예측 및 확인하지 않았다. 각 service페이지를 일일히 하나하나 보고 전처리 함수를 만들고 하다보니 전반적인 scope를 예상하지 못하여 함수 크기가 점점 커지고 복잡도가 증가한 것이다.
문제 발생
조회를 할 때 제대로 된 값이 나오지 않는 다던가 조회가 되었는데 클릭해서 특정 데이터를 다시 조회하고자 하니 제대로 동작하지 않는다던가 하는 문제가 발생했다. 이렇게 순환적으로 문제가 발생했고 원인을 찾아보니 위와같이 함수의 복잡도가 매우 커서 예외를 제대로 처리 하지 못햇고 올바르지 않은 쿼리가 나간 것이 문제였다.
팀원들과 코드 리뷰를 했을 때 팀원도 내가 만든 함수가 정확히 무엇인지 한번에 파악하기 힘들다 하였고 어떻게 활용되고 있는 것인지 이해하기 힘들다고 하였다.
해결 방법
필터링과 where 구문 생성을 어떻게 하는 것이 좋을까 생각 해보았다.
-
기존의 함수를 쪼개 함수의 역할 범위를 좁히자 -> 각 서비스마다 전체 필터를 처리하는 함수를 각각 만들자
-
각 where문을 만드는 service에서 일일히 다 새로 처리하자
-
preprocessingValue처럼 기존의 전처리 함수를 이용해 각 서비스에 적용시키자
-
기존의 함수가 제대로 안되는 것은 특정 값만 제대로 처리하지 못해서 제대로 안된 것 같으니 해당 기존 함수를 유지하되 수정을 하자
-
각 filter 처리하는 공통적인 부분만을 뽑아내어 각 함수를 만들어 필요 상황에 따라 그것을 각 service 에서 사용하자
4가지의 방식이 존재했는데 내가 선택한 방식은 5번 이었다.
선택 이유
-
각 service에 전체 필터를 처리하는 함수를 만들어 활용하는 것은 전체 페이지가 10페이지가 넘어가고 처리해야하는 filter양이 너무 많고 각 필터 처리하는 부분이 다양하긴 하더라도 반복해서 사용하는 것도 많기 때문에 비효율 적이라고 생각했다.
-
1번의 이유와 같이 반복되는 값들을 효율적 처리 하지 못함
-
기존 preprocessing 함수에서 문제가 발생했고 해당 preprocessing의 복잡도가 상당히 높고 헷갈리기
때문에 기존 코드를 재사용한다면 유지보수에서 어려움이 발생 할 수 있다. 예를 들어 특정 key가 들어오면 새로운 전처리 로직을 추가할 때 다른 곳에서 영향도가 없는지 확인하기 어려울 수 있다. -
기존의 함수는 유지 보수가 어렵고, 가독성이 매우 떨어진다. 직관적이지 못해 함수의 재사용가능성이 떨어질 확률이 높다. 또한 함수의 길이와 args가 많아 실수가 발생할 가능성 즉 side effect 발생 가능성이 매우 크다.
5. 유지 보수가 쉽고 함수의 복잡도가 떨어져서 가독성이 매우 크다. 또한 만약 새로운 유형의 filter처리가 필요하다면 class에 그런 특정 유형의 필터 처리하는 함수를 만들면 되기 때문에 확장성 또한 높다고 판단하였다.
결과
5번의 해결방법을 사용하여 각 filter를 공통적으로 처리하는 함수를 만들고 그것을 각 service에서 filter을 이용해 where문을 드는 곳에 적용시켰다. 6시간동안 함수를 다 고치고 리팩토링 했더니 수많은 side effect가 거의 없어졋다.
반성
앞으로 이렇게 큰 범위의 코드 수정이 필요할 때 전반적으로 내가 수정해야할 범위 체크를 먼저 한다.
함수의 크기 함수의 복잡도를 고려하자 이전 clean code에서 읽었던 글이 하나 떠오른다. 함수의 크기는 4?줄이 넘지 않아야하고
args의 양은 4개 아래가 되어야한다가 생각났다. clean code에서 하는 말이 전부 맞다고 생각하진 않지만 어느정도 고려해야 겠다고 생각했다. 이래서 이 사람이 args와 함수 크기를 줄이라고 생각한건가 라는 생각이 들었다. 등등 이것저것 생각나는 것이 많았는데
주말에는 좀 쉬어야지... 작성하고 헤드퍼스트 디자인패턴, effective typescript나 한번읽고 nest다시 공부하러가야겟다.
