Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Creating pull request returns 500 error because of commit symbol #14470

Closed
1 of 6 tasks
lakostin opened this issue Jan 26, 2021 · 6 comments · Fixed by #14624
Closed
1 of 6 tasks

Creating pull request returns 500 error because of commit symbol #14470

lakostin opened this issue Jan 26, 2021 · 6 comments · Fixed by #14624
Labels
type/bug
Milestone
1.13.3

Comments

@lakostin
Copy link

  • Gitea version (or commit ref):
    1.14.0+dev-534-gedbc5c86d
  • Git version:
    2.26.2
  • Operating system:
    docker Alpine Linux v3.12
  • Database (use [x]):
    • PostgreSQL
    • MySQL
    • MSSQL
    • SQLite
  • Can you reproduce the bug at https://try.gitea.io:
    • Yes (provide example URL)
    • No
  • Log gist:
    2021/01/26 14:11:06 ...ters/repo/compare.go:477:PrepareCompareDiff() [E] GetCommit: encoding/hex: invalid byte: U+004B 'K'

Description

...

Screenshots

Screenshot from 2021-01-26 14-18-12
@lunny
Copy link
Member

lunny commented Jan 26, 2021

If is this repository a public one?

@lakostin
Copy link
Author

If is this repository a public one?

private

@lunny lunny added the type/bug label Jan 28, 2021
@zeripath
Copy link
Contributor

zeripath commented Feb 9, 2021

I bet the issue is that the name of the branch contains exactly 40 characters

zeripath added a commit to zeripath/gitea that referenced this issue Feb 9, 2021
@zeripath
Do not assume all 40 char strings are SHA1s
7c2a779 
GetCommit() assumes that all 40 char strings are SHA1s. This leads to an
error if you try to do a PR on a branch which is 40 characters long.

This PR attempts the SHA first - and if it fails will switch to using rev-parse.

Fix go-gitea#14470

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath mentioned this issue Feb 9, 2021
Merged
@lunny lunny added this to the 1.13.3 milestone Feb 10, 2021
@zeripath
Copy link
Contributor

Apologies for this I've predicted this issue for a very long time but haven't told anyone about it or fixed it.

The provided fix is by no means perfect as a branch whose name matches the sha pattern could still collide in interesting and undefined ways and so we will need deeper testing to figure out what we want.

zeripath added a commit that referenced this issue Feb 10, 2021
@zeripath
Do not assume all 40 char strings are SHA1s (#14624)
0a23079 
GetCommit() assumes that all 40 char strings are SHA1s. This leads to an
error if you try to do a PR on a branch which is 40 characters long.

This PR attempts the SHA first - and if it fails will switch to using rev-parse.

Fix #14470

Signed-off-by: Andrew Thornton <art27@cantab.net>
@richmahn
Copy link
Contributor

@zeripath this is also on the v1.13.3 milestones so needs a backport.

@zeripath
Copy link
Contributor

zeripath commented Feb 10, 2021 •
edited
Loading

Feel free to send some of these backports. I don't mind.

Done it now

zeripath added a commit to zeripath/gitea that referenced this issue Feb 11, 2021
@zeripath
Do not assume all 40 char strings are SHA1s (go-gitea#14624)
ae8233e 
Backport go-gitea#14624

GetCommit() assumes that all 40 char strings are SHA1s. This leads to an
error if you try to do a PR on a branch which is 40 characters long.

This PR attempts the SHA first - and if it fails will switch to using rev-parse.

Fix go-gitea#14470

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath mentioned this issue Feb 11, 2021
Merged
lunny pushed a commit that referenced this issue Feb 13, 2021
@zeripath
Do not assume all 40 char strings are SHA1s (#14624) (#14648)
101fb0d 
Backport #14624

GetCommit() assumes that all 40 char strings are SHA1s. This leads to an
error if you try to do a PR on a branch which is 40 characters long.

This PR attempts the SHA first - and if it fails will switch to using rev-parse.

Fix #14470

Signed-off-by: Andrew Thornton <art27@cantab.net>
@go-gitea go-gitea locked and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
type/bug
Projects
None yet
Milestone
1.13.3
Development

Successfully merging a pull request may close this issue.

Do not assume all 40 char strings are SHA1s zeripath/gitea
4 participants
@lunny @zeripath @richmahn @lakostin