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.

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

Gitea Webhook - wrong FilePath #15865

Closed
1 task
msklenicka opened this issue May 13, 2021 · 3 comments · Fixed by #16288
Closed
1 task

Gitea Webhook - wrong FilePath #15865

msklenicka opened this issue May 13, 2021 · 3 comments · Fixed by #16288
Labels

Comments

@msklenicka
Copy link

Some time ago, i wrote on gitea forum:
https://discourse.gitea.io/t/webhook-sends-wrong-incomplete-file-path/2823/4

If we used webhook gitea feature, we discovered one “strange” behavior. When Path to modified file contains SPACE, then gitea sends incomplete path in commits.modified.
Here is JSON example:

{
“secret”: “”,
“ref”: “refs/heads/master”,
“before”: “aed0bef8b7ef905893e815ebf2a25bceaddc159d”,
“after”: “66069ec2616ce7f00a5722561cf38dd91b47112e”,
“commits”: [
{
“id”: “66069ec2616ce7f00a5722561cf38dd91b47112e”,
“message”: “Gitea Webhook test\n”,
“url”: “https://gitea.local/TEST/Core/commit/66069ec2616ce7f00a5722561cf38dd91b47112e”,
“author”: {
“name”: “msklenicka”,
“email”: “msklenicka@mymail.local”,
“username”: “msklenicka”
},
“committer”: {
“name”: “msklenicka”,
“email”: “msklenicka@mymail.local”,
“username”: “msklenicka”
},
“verification”: null,
“timestamp”: “2021-01-05T10:45:28+01:00”,
“added”: [],
“removed”: [],
“modified”: [
“Java”
]
}
]… }

This is proper filepath: Java Source/jmesa.properties

We needed to fix this behavior, so we made our own release.
This is how we fixed it:
File: gitea/modules/git/commit.go

commit go_fix

Gitea version: 1.14.1

webhook0
webhook1

@zeripath
Copy link
Contributor

Yes this looks like it would fix the bug - do you want to send up a PR?

@noerw noerw added the type/bug label May 13, 2021
@msklenicka
Copy link
Author

msklenicka commented May 18, 2021 via email

@zeripath
Copy link
Contributor

Sorry I missed this.

I think there's a bit more of a problem here and we need to use:

git log --name-status -c --pretty=format:'' --parents --no-renames -t -z -1 $COMMIT_ID

and use the same style of parsing developed in LogNameStatusRepo

zeripath added a commit to zeripath/gitea that referenced this issue Jun 28, 2021
There is an unfortunate bug with GetCommitFileStatus where files with
spaces are misparsed and split at the space.

There is a second bug because modern gits detect renames meaning that
this function no longer works correctly.

There is a third bug in that merge commits don't have their modified
files detected correctly.

Fix go-gitea#15865

Signed-off-by: Andrew Thornton <art27@cantab.net>
6543 pushed a commit that referenced this issue Jul 2, 2021
* Fix modified files list in webhooks when there is a space

There is an unfortunate bug with GetCommitFileStatus where files with
spaces are misparsed and split at the space.

There is a second bug because modern gits detect renames meaning that
this function no longer works correctly.

There is a third bug in that merge commits don't have their modified
files detected correctly.

Fix #15865


Signed-off-by: Andrew Thornton <art27@cantab.net>
AbdulrhmnGhanem pushed a commit to kitspace/gitea that referenced this issue Aug 10, 2021
…6288)

* Fix modified files list in webhooks when there is a space

There is an unfortunate bug with GetCommitFileStatus where files with
spaces are misparsed and split at the space.

There is a second bug because modern gits detect renames meaning that
this function no longer works correctly.

There is a third bug in that merge commits don't have their modified
files detected correctly.

Fix go-gitea#15865


Signed-off-by: Andrew Thornton <art27@cantab.net>
@go-gitea go-gitea locked and limited conversation to collaborators Oct 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants