-
Notifications
You must be signed in to change notification settings - Fork 62
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
bnu
committed
Oct 16, 2015
1 parent
87b07e8
commit 3e34fd0
Showing
1 changed file
with
1 addition
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
3e34fd0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bnu
이 패치를 적용한 시점에 default_url이 공백이었던 사용자는 문제가 발생됩니다.
본래의 도메인으로 설정도 할 수 없을 뿐더러, 관리자로의 로그인도 불가능 하게 됩니다.
저는 이것이 코어의 문제라고는 보지 않지만 사용자에게 위험성을 고지할 필요가 있다고 생각합니다.
또, 제가 만든 멀티도메인 모듈은 이 패치로 현재 배포버전은 사용이 안되지만 모듈을 수정함으로써 정상 동작 합니다. 그리고 위에 적은 default_url 문제도 멀티도메인 모듈이 설치되었다면 해결됩니다. 제 모듈도 패치 할테니 승인요청 확인되시면 승인 드리겠습니다. :)
3e34fd0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bnu
패치 올렸습니다. 승인 부탁합니다. :p
승인 드리겠습니다. 라니... 승인 부탁 드리겠습니다. ^^
3e34fd0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bnu
//check CSRF for admin actions
주석도... 수정해야겠네요.
3e34fd0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
default_url이 지정되지 않은 경우에는 그냥 현재의
$_SERVER['HTTP_HOST']
값을 기준으로 체크하면 안되나요? 리퍼러 도메인과 현재 페이지의 도메인이 일치하기만 하면 되잖아요. 괜히 복잡하게 생각하는 게 아닌가 합니다.