쏚쏚스포이드

최근 팀원과 프로젝트 진행 중 배포 후 구글폼과 함께 배포된 사이트를 피드백받고자 홍보를 했었다.

홍보 후 들어왔던 피드백중 하나였는데,

“검색창에 select를 치면 모든 게시글이 나와요.“

라는 피드백이였고, 너무 놀란 나머지 최대한 빨리 원인분석을 해보았다. 원인분석 이후 도달한 결론은 “문제없음” 이였다.

이유는 우리의 injection방어로직은 아래와 같은데,

    /**
     * SQL Injection 방어하기 위하여 구현
     */
    private String changeNoSqlInjection(String query) {
        query = SPECIAL_CHARS.matcher(query).replaceAll("");
        for (String s : STRING_SET) {
            if (query.contains(s)) {
                return "";
            }
        }
        return query;
    }

해당 코드에서는 sql문인 select, update, insert, delete의 문자가 들어오면 무조건 빈 문자열을 리턴하기 때문에 “select”를 포함한 문자열을 검색했을 시 빈 문자열로 검색을 돌렸기에 위와 같은 상태가 되었던 것이었기에 문제가 없는 것이다.

하지만 이번 일로 인해 프로젝트를 진행하면서 중요하게 놓치고 있는 것을 다시한번 깨닫게 되었는데, 이유는 아래와 같다.

• 내가 팀원이 구현한 해당 sql Injection관련 코드를 봤었는데 그냥 그렇구나~ 하고 가볍게 넘긴 것
• 해당 기능이 구현된 후 직접 테스트해보지 않은 것

평소에 코드리뷰를 하려고 pull request 후에 리뷰, approve하는 형식인데 어느순간부터 아무 생각없이 approve만 하는 습관이 생기는 것 같다 물론 내가 바쁜 것도 바쁜 것이지만, 상대방의 코드를 통해 배울 점도, 부족한 점이 있다면 서로 피드백을 하면서 확인하는 것이 내가 성장하기에 가장 좋을텐데 간과하고 있었던 것이 이렇게 돌아올줄이야… 또, SQL injection이라면 기본적이면서도, 중요한 문제인데 이를 구현된 코드만 보고 가볍게 넘어가고 테스트해보지 않은 것이 문제가 있다고 생각했고, 더 신경써서 테스트를 해봐야 한다고 다시한번 느꼈다.

@RequiredArgsConstructor를 처음엔

그냥 생성자를 알아서 주입해주는구나~ 생각만 가볍게 해두고 공식처럼 사용해왔다.

발단

그렇게 스프링부트 프로젝트를 협업을 통해 진행한지 좀 지났을 때

리팩토링을 위해 코드를 개선하다가 문제가 발생했다.

하나의 도메인에서 다른Repository를 불러오는 것이 응집도를 올린다고 생각했고, 또 Repository에 직접 접근하는 것은 막아야 한다고 생각했기에 xxxxRepository를 그대로 써오는 것에서 xxxxService로 변경해 사용하는 것으로 바꿔 생각했다.

이 과정에서 우리는 Service부분의 클래스 생성을 xxxxRepository → xxxxService로 변경, 생성자 주입을 시켰는데 문제가 발생했다..!

결론부터 말하자면 @RequiredArgsConstructor의 특성을 제대로 생각하지 못한 것이 문제였고 자세한 이유는 아래에서 다루겠다.

그래서

처음에 원인 파악을 할 때 해당 클래스들의 생성자 주입에서 final이 빠졌다고 인지를 아예 못 한 상황이였고, 때문에 한참을 뻘짓 한 것 같다.

그 때 당시에 코드변경 후 직접 실행해 확인하지 않고, 바로 테스트코드를 작성하고 있어서 해당 UserService 자체가 문제라고 인지하지 못했고,

테스트코드 작성도 익숙치 않은 상태라 테스트코드를 못짜서 그렇다고 판단을 했고, 테스트 환경을 좀 더 공부하는 뻘짓(?)을 했고, 해당 뻘짓을 할 때 디버그를 통해 알아낸 정보는 아래의 사진과 같았다.

Service에 있는 클래스가 모두 null로 되어있는 것… 이걸 발견하고도 한참 뻘짓하기도 하고 기초중의 기초적인 실수였지만....

그래도! 발견한 것에 의미를 두자! 라고 생각을 하기로 했다.

정리해서 말하자면 @RequiredArgsConstructor 어노테이션의 특성이 생성자 주입 시 final선언을 해줘야 자동으로 생성자 주입을 하고 스프링 빈을 생성하는데, final이 없어 생성자 주입을 하지 않았기 때문에 각 Service들이 null상태로 있었던 것이고, 이러한 문제가 발생했었던 것이다.

기존 UserService.java 생성자주입 예시

private final UserRepository userRepository;
private final UserTagRepository userTagRepository;
private final FriendRepository friendRepository;

xxxxService로 변경했을 때 문제가 있던 코드 필드

private final UserRepository userRepository;
private UserTagService userTagService;
private FriendService friendService;

 또 만약에 또 이런 문제가 생겼을 때 디버그를 통해 필드객체가 null로 되어있다면 final선언을 했는지 한번 더 확인할 수 있는 계기가 되었다.