2 posts tagged with "pull-request"

이전에 pull request 한 것에 대한 메일이 왔다.

1. Commit에 대한 comment

누군가가 github에서 내가 올려둔 pull request에 comment를 달았는데 같은 내용이 메일로 온 것이다. 내용은 내가 수정할 때 사용한 ENT_IGNORE라는 option (flag)가 보안 이슈가 있는 건데 사용한 특별한 이유가 있냐는 것이었다. 앞서도 언급했지만 난 PHP를 잘 모른다. Flag list들을 문서에서 참고했을 때 있으면 좋을 것이라고 생각했던 것이었는데 그게 보안 이슈가 있는지는 미처 몰랐다.

추가로 comment 단 사람이 관련 문서도 함께 수정해 달라고 요청했다. 문서가 있는지도 몰랐네.

그리고 contribution 해준게 고맙단다. 사실 나도 고맙다. 내 입장에서는 경험을 쌓는건데 그걸 또 고맙다고 하니까. 뭔가 훈훈하다.

2. 정보 수집

제기된 보안 이슈는 정말 그런 것일까? 문제가 된 ENT_IGNORE에 대한 자료를 찾아보자. 우선 사용한 함수 htmlentities에 대한 문서부터 다시.

아. 문서(http://kr1.php.net/htmlentities) 자체에도 언급이 되어있다. may have security implications 라고. 왜 미처 저 부분을 못 봤는지...(공식 API 문서에도 언급된 부분이라 신뢰도는 높다고 판단된다) 보안 이슈에 대해 더 자세히 확인하고 싶지만 생략하기로 한다. 기술적인 부분 보다는 open source 활동과 과정에 대한 정리가 지금은 중요하다고 생각하기 때문이다.

3. 작업

어찌되었든 혹시나 해서 넣어둔 flag인데 빼도 문제가 없는지 검토해 보기로 한다. 제거하는 것으로 코드를 수정한 후에 문제가 없는지 테스트를 진행했다. 다행히 ENT_IGNORE를 제거함으로 인해 발생하는 문제는 없었는데 또 다른 문제를 발견할 수 있었다. AChecker가 XML을 생성하면서 정의되지 않은 entity가 포함되는 경우가 있었는데 이에 대한 정의가 없었기 때문에 정상적인 XML로 인식되지 못했다. 그래서 보너스로 그 부분까지 함께 수정. 그리고 commit / push.

4. Pull request에 대한 update

이전에 올린 pull request를 제거하고 다시 등록해야 하나 고민을 했다. 고민을 하면서 최소한 이전에 comment 단 사람에게 댓글은 달아줘야겠다는 생각이 들어서 github에 가보니...

 

이전의 comment 아래에 내가 fork한 저장소로 push 했던 수정사항이 자동으로 달려있다. Comment에 수정할 파일과 라인넘버까지 명시되어 있었기 때문에 그 부분을 수정한 commit을 자동 감지하는 것인지, 원래 pull request로 등록한 branch에 대한 모든 변경사항들이 pull request가 close 되기 전까지 자동으로 update 되는게 맞는지는 좀 더 살펴봐야 하겠지만 일단 예상했던 자잘한 작업을 안해도 되기 때문에 무척 편리하다는 생각이 들었다.

마지막으로 수정한 부분에 대한 comment를 달아줬다. 영작이 이상하더라도 문제 삼지는 말자. 원래 영어를 잘하지도 못하고 native도 아닌데 우선은 의사전달만 제대로 되면 큰 문제는 없다고 본다. 그래도 영어공부는 계속 해둬야 겠다는 생각은 한다.

 

역시 이 작업까지 마치고 다음을 기다리기로 했다.

웹 접근성 관련 작업을 할 일이 생겨서 AChecker라는 서비스를 살펴보게 되었다.

AChecker는 사용자가 입력한 URL에 대해서 지정한 웹접근성 Guideline(이하 WAG)을 준수하고 있는지 검사를 해주는데, 기본으로는 HTML 형태로 출력해주지면 감사하게도 REST API 역시 제공하고 있다. REST API를 사용할 경우 Output type은 XML 뿐이다.

1. 문제점에 대한 인지

사용해보면서 기능에 대해 몇가지 검토를 하는 도중에 검사 대상이 되는 페이지에 한글이 포함되어 있는 경우 XML 이 깨지는 문제를 발견하게 되었고 좀 더 명확한 문제의 원인을 알기 위해 소스코드를 살펴보기로 했다. AChecker의 소스코드는 Github에 등록되어 있어서 브라우저로 소스를 살펴보기가 나름 편하다. 깔끔하기도 하고.

PHP로 되어있는 코드를 이리저리 살펴보니 (난 PHP를 잘 모른다. 하지만 상세한 문법과 함수들을 모르더라도 훑어보고 문제가 될만한 부분을 찾는데에는 문제가 없다고 생각했다) REST API에 대한 클래스의 인코딩이 조금 이상하다는 걸 발견할 수 있었다. (ISO-8859-1 이었음) EUC-KR이나 UTF-8이 아니라면 한글 표시에 문제가 발생할 수 있겠다 생각을 하면서 REST 말고 HTML 생성하는 부분도 살펴보니 HTML은 UTF-8로 생성하고 있었다. 문제가 되는 URL에 대한 검사 결과를 비교해보니 역시나 HTML로 output을 받을 때에는 전혀 문제가 발생하지 않았고 최종적으로 encoding이 원인이라는 것을 확신하게 되었다. 또 어디를 수정해야 하는지도... (PHP를 몰라도 검색을 좀 해보니 해결책을 찾기가 어렵진 않았음)

2. Contribution을 하자

내가 업무에 쓸거니 그냥 나만 수정해서 쓸까 했는데, 예전부터 마음먹고 있던 Open source 활동을 시작하는 기회로 만들면 어떨까하는 생각이 들었고 별 고민없이 진행하기로 했다. 어차피 수정할 내용이 많지도 않아서 시간 소모가 작을 것 같다는 생각이 마음을 좀 더 편안하게 해주었다.

3. Fork

우선은 AChecker 저장소를 fork 하기로 했다. Github에서 fork는 원래의 저장소를 따와서 내 계정에 동일한 저장소를 만드는 과정을 의미한다. Fork를 하니 아래의 그림처럼 내 계정에 동일한 저장소가 만들어진다.

4. 작업

Fork한 저장소를 clone해야 내가 수정할 수 있다. 물론 원래의 저장소를 clone해서 수정해도 되겠지만 pull request를 할 것이므로 fork한 저장소를 clone 해야 한다. SSH로 clone도 하고 push 하는게 편하기 때문에 미리 github에 public key를 등록해 주었다. 그 이후엔 내가 쓰는 환경에서 코드를 수정했고, 테스트를 해봤다. 잘 된다. 문제가 해결되었다. 그 다음엔 commit. 내가 등록하는 commit message가 그대로 남기 때문에 혹시나 AChecker만 가지고 있는 commit message rule 같은게 없는지 살펴봤는데 제약사항이 없는 것 같았으므로 적당히 알아볼 수 있도록 message를 등록했다. 그리고 push해서 fork한 내 저장소에 commit이 제대로 올라갔는지 확인.

5. Pull request

Fork한 저장소에는 반영되었으니 이제 pull 해가라고 요청해야 한다. AChecker 저장소 페이지로 이동해 Pull Requests 메뉴로 가보니 새로 등록하는 버튼이 있다. (아래 그림 오른쪽 상단 녹색 버튼) 버튼을 누르니 fork한 저장소에 등록된 commit message가 title에 입력되어 있다. 적당히 제목과 내용 적어서 완료.

등록하고 나면 Pull Requests list에 표시되고 Issues에도 함께 나타난다.

이제 결과만 기다리면 되겠다.