Skip to content

Conversation

mrsdizzie
Copy link
Member

Clean up checks on reference names to better conform to the guideline
here: https://git-scm.com/docs/git-check-ref-format

This fixes half of #6321

To see example of current problem, just try and create a branch or tag with a valid name containing allowed special characters like test+me. It will incorrectly say TagName must be a well-formed Git reference name

I've tried to add several extra tests as well since if a bad name gets by this check it can later through a less friendly 500 error if we try to create a branch with a bad name

Clean up checks on reference names to better conform to the guideline
here: https://git-scm.com/docs/git-check-ref-format

This fixes half of go-gitea#6321
According to: https://git-scm.com/docs/git-check-ref-format

And: git check-ref-format "master/feature=test1"

This is a valid branch name and we should not be testing for it to fail.
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Mar 26, 2019
@lafriks lafriks added this to the 1.9.0 milestone Mar 26, 2019
@GiteaBot GiteaBot 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 26, 2019
@lafriks lafriks merged commit d056bf3 into go-gitea:master Mar 26, 2019
@codecov-io
Copy link

Codecov Report

Merging #6437 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6437      +/-   ##
==========================================
- Coverage   38.85%   38.85%   -0.01%     
==========================================
  Files         365      365              
  Lines       51413    51414       +1     
==========================================
- Hits        19978    19977       -1     
- Misses      28566    28568       +2     
  Partials     2869     2869
Impacted Files Coverage Δ
routers/repo/branch.go 58.19% <100%> (ø) ⬆️
modules/validation/binding.go 100% <100%> (ø) ⬆️
modules/auth/auth.go 50.4% <0%> (-0.81%) ⬇️

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 b4941f7...aced5e7. Read the comment docs.

@lafriks
Copy link
Member

lafriks commented Mar 26, 2019

Please send backport to release v1.7 and v1.8 branches

mrsdizzie added a commit to mrsdizzie/gitea that referenced this pull request Mar 26, 2019
* Clean up ref name rules

Clean up checks on reference names to better conform to the guideline
here: https://git-scm.com/docs/git-check-ref-format

This fixes half of go-gitea#6321

* Update branch create integration test

According to: https://git-scm.com/docs/git-check-ref-format

And: git check-ref-format "master/feature=test1"

This is a valid branch name and we should not be testing for it to fail.
@techknowlogick techknowlogick added the backport/done All backports for this PR have been created label Mar 26, 2019
mrsdizzie added a commit to mrsdizzie/gitea that referenced this pull request Mar 26, 2019
* Clean up ref name rules

Clean up checks on reference names to better conform to the guideline
here: https://git-scm.com/docs/git-check-ref-format

This fixes half of go-gitea#6321

* Update branch create integration test

According to: https://git-scm.com/docs/git-check-ref-format

And: git check-ref-format "master/feature=test1"

This is a valid branch name and we should not be testing for it to fail.
zeripath pushed a commit that referenced this pull request Mar 26, 2019
@zeripath zeripath mentioned this pull request Mar 26, 2019
3 tasks
@lunny
Copy link
Member

lunny commented Mar 27, 2019

@techknowlogick since #6321 marked as v1.7.5 I think we should also back port to v1.7 forget my words

@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/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants