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

New pull request from last pushed branch 404 error #25830

Closed
stuzer05 opened this issue Jul 11, 2023 · 15 comments · Fixed by #26744
Closed

New pull request from last pushed branch 404 error #25830

stuzer05 opened this issue Jul 11, 2023 · 15 comments · Fixed by #26744
Assignees
@puni9869
Labels
issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail type/bug

Comments

@stuzer05
Copy link

stuzer05 commented Jul 11, 2023 •
edited
Loading

Description

We use issue reference as branch name
image

and when I pushed my branch, gitea suggested to create new pr, however "#" in the branch name breaks this feature
image
image

Also, pushed branches from one repository are shown in another one for some reason

Gitea Version

trunk

Can you reproduce the bug on the Gitea demo site?

Yes

Log Gist

No response

Screenshots

No response

Git Version

No response

Operating System

No response

How are you running Gitea?

docker

Database

None
@puni9869
Copy link
Member

puni9869 commented Jul 11, 2023 •
edited
Loading

This is related to html encoding, url.PathEscape of # in the url . I can replicate it in my local.

@puni9869 puni9869 self-assigned this Jul 11, 2023
@puni9869 puni9869 mentioned this issue Jul 12, 2023
Closed
@puni9869
Copy link
Member

I need some feedback on above Pr.

@stuzer05
Copy link
Author

I need some feedback on above Pr.

Works with the branch name, however these messages are shown for the wrong repo
image

@puni9869
Copy link
Member

I need some feedback on above Pr.

Works with the branch name, however these messages are shown for the wrong repo image

I am little bit confused.
Q 1. Do you have repo1 as a repository in your gitea account.
or
Q 2. Do you have repo1 as a branchname in the repo2 repository [the current repository.]

PS: Please share the screenshot of branches list of repo2 repository. I think these are two different Issues.
Let reproduce one by one.

@puni9869 puni9869 added the issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail label Jul 12, 2023
@stuzer05
Copy link
Author

I have 2 repos and issue branch in each of them, created from master
image
image

@puni9869
Copy link
Member

Works with the branch name, however these messages are shown for the wrong repo image

Technically branches should be from same repo only.
I have less knowledge on this feature. Is it intentional to show all branches from all repositories.

PS: I have confirmed, github shows only branches which are to beNew Pull Request only from that repository only.

@stuzer05
Copy link
Author

I have less knowledge on this feature. Is it intentional to show all branches from all repositories.

This must be a bug, because in the code of those messages there're 2 sql queries WITH repository check. But it works incorrectly

@puni9869
Copy link
Member

Fix: #25812

@stuzer05
Copy link
Author

Fix: #25812

You didn't include fix for branch names with "#" here

@puni9869 puni9869 mentioned this issue Jul 13, 2023
Merged
@yp05327 yp05327 mentioned this issue Jul 13, 2023
Closed
@stuzer05
Copy link
Author

Branch selector also doenst work
image
image

But when you replace "#" with "%23" in url, everything is okay. Branch name should be url encoded in all "a" tags

@yp05327
Copy link
Contributor

yp05327 commented Jul 14, 2023

Branch selector also doenst work image image

But when you replace "#" with "%23" in url, everything is okay. Branch name should be url encoded in all "a" tags

Will be fixed in #25875

@lunny
Copy link
Member

lunny commented Jul 27, 2023

Close per #25875 merged.

@lunny lunny closed this as completed Jul 27, 2023
@stuzer05
Copy link
Author

@lunny please, reopen. Error is still not fixed on latest dev build.

I have a branch named test/test#123
image

  1. Branch name insede the message is still escaped, but shouldn't
  2. In "New Pull Request" button "#" is escaped as "%2523", which is incorrenct. Correct escaping is "%23" as in branch name inside the message (showed by an arrow)

I tested if button would lead to url with correct %23 escaping everything would work

@lunny lunny reopened this Aug 11, 2023
@puni9869
Copy link
Member

This is due to double encoding. Its a known issue.
cc @yp05327

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Aug 26, 2023 •
edited
Loading

The reported bug is not related to #25875

-> Fix template bugs in recently_pushed_new_branches.tmpl #26744

@wxiaoguang wxiaoguang mentioned this issue Aug 26, 2023
Merged
wxiaoguang added a commit that referenced this issue Aug 27, 2023
@wxiaoguang
Fix template bugs in recently_pushed_new_branches.tmpl (#26744)
5914a2f 
Fix some bugs from #25715, fix #25830

1. `$.locale.Tr ... Safe` needs `Escape`, but not `PathEscapeSegments`
2. The attribute should be `role`
3. The `ComposeBranchCompareURL` already does escaping correctly
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@puni9869 puni9869

Labels
issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail type/bug
Projects
None yet
Milestone
No milestone
5 participants
@lunny @wxiaoguang @yp05327 @stuzer05 @puni9869