Skip to content

Conversation

lafriks
Copy link
Member

@lafriks lafriks commented Mar 15, 2018

Refactor and simplify redirect to url, also improve checking url for errors

@lafriks lafriks added type/refactoring Existing code has been cleaned up. There should be no new functionality. backport/v1.4 labels Mar 15, 2018
@lafriks lafriks added this to the 1.5.0 milestone Mar 15, 2018
@codecov-io
Copy link

Codecov Report

Merging #3674 into master will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@ Coverage Diff @@ ## master #3674 +/- ## ========================================== - Coverage 35.87% 35.87% -0.01%  ========================================== Files 287 287 Lines 41362 41359 -3 ========================================== - Hits 14840 14837 -3  + Misses 24336 24334 -2  - Partials 2186 2188 +2
Impacted Files Coverage Δ
routers/user/auth.go 15.01% <0%> (+0.05%) ⬆️
routers/user/auth_openid.go 0% <0%> (ø) ⬆️
modules/context/context.go 43.55% <0%> (-3.16%) ⬇️
routers/repo/repo.go 22.75% <0%> (+0.3%) ⬆️
routers/user/profile.go 33.49% <0%> (+0.63%) ⬆️
modules/process/manager.go 76.81% <0%> (-4.35%) ⬇️
models/repo_indexer.go 48.3% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a2a49c9...a585d2e. Read the comment docs.

@tboerger tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 15, 2018
@tboerger tboerger added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 15, 2018
@tboerger tboerger added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 15, 2018
}

u, err := url.Parse(loc)
if err != nil || (u.Scheme != "" && !strings.HasPrefix(strings.ToLower(loc), strings.ToLower(setting.AppURL))) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will probably break someones setup, but it's slightly more secure 🤷‍♂️

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would it break?

@bkcsoft bkcsoft merged commit 7b2b900 into go-gitea:master Mar 15, 2018
@lafriks lafriks deleted the feat/refactor_redirect_to branch March 15, 2018 22:54
lafriks added a commit to lafriks-fork/gitea that referenced this pull request Mar 15, 2018
@lafriks lafriks added the backport/done All backports for this PR have been created label Mar 15, 2018
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/refactoring Existing code has been cleaned up. There should be no new functionality.

5 participants