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

Incorrect link for submodules that specify the remote port #2775

Closed
2 of 7 tasks
Governa opened this issue Oct 24, 2017 · 12 comments · Fixed by go-gitea/git#136 · May be fixed by go-gitea/git#108
Closed
2 of 7 tasks

Incorrect link for submodules that specify the remote port #2775

Governa opened this issue Oct 24, 2017 · 12 comments · Fixed by go-gitea/git#136 · May be fixed by go-gitea/git#108
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/bug

Comments

@Governa
Copy link

Governa commented Oct 24, 2017 •
edited
Loading

Description

In the example URL, there is a submodule called test214 that uses the port (22) as user name in its link.
The submodule configuration is valid and works on the command line.
@lafriks lafriks added this to the 1.x.x milestone Oct 24, 2017
@Governa
Copy link
Author

Governa commented Jan 18, 2018

This one (superficially) seems to be pretty simple. If anyone can point me to the right direction I could solve it myself.

@thehowl
Copy link
Contributor

thehowl commented Jan 18, 2018

The culprit should be this function here: https://github.com/go-gitea/git/blob/master/submodule.go#L32-L74 Have fun! If you need any help, come on #develop on the Discord server or #gitea-dev on Freenode :)

Governa pushed a commit to Governa/git that referenced this issue Jan 26, 2018
Fixes incorrect url for submodules that specify the remote port
b2c17ef 
  Fixes go-gitea/gitea#2775
Governa pushed a commit to Governa/git that referenced this issue Jan 26, 2018
Fixes incorrect url for submodules that specify the remote port
5c8129c 
  Fixes go-gitea/gitea#2775

Signed-off-by: Fernando Governatore <[email protected]>
Governa pushed a commit to Governa/git that referenced this issue Jan 26, 2018
Fixes incorrect URL for submodules that specify the remote port
a80f846 
  If the repo's host is in urlPrefix, test if there is a number after the
":" and, if there is one, consider it as a port number and ignore it in the
URL returned.

  Fixes go-gitea/gitea#2775

Signed-off-by: Fernando Governatore <[email protected]>
@Governa Governa mentioned this issue Jan 26, 2018
Open
Governa pushed a commit to Governa/git that referenced this issue Jan 26, 2018
Fixes incorrect URL for submodules that specify the remote port
0ef46ae 
  If the repo's host is in urlPrefix, test if there is a number after the
":" and, if there is one, consider it as a port number and ignore it in the
URL returned.

  Fixes go-gitea/gitea#2775

Signed-off-by: Fernando Governatore <[email protected]>
@lunny lunny mentioned this issue Dec 9, 2018
Merged
@lafriks lafriks removed this from the 1.x.x milestone Jan 23, 2019
@Governa
Copy link
Author

Governa commented Feb 6, 2019

Hi,

Is this fix on the Docker image already? I've tested commit "da1edbf" here and it doesn't seems to solve the problem. Is there anything I should do to "update" the links?

@lunny
Copy link
Member

lunny commented Feb 7, 2019

This has been merged to latest Docker image I think.

@Governa
Copy link
Author

Governa commented Apr 17, 2019 •
edited
Loading

The problem persists. The submodule link shows http(s)://<url>/<port>/<repo> instead of http(s)://<url>:<port>/<repo>.

The workaround I'm using is to set a relative path on the submodule URL. Fortunately, the workaround is the correct way of doing things 👍, but older revisions still have the wrong link.

@zeripath
Copy link
Contributor

Hi Governa, could you upload another testcase to try, so I can check this out?

@zeripath zeripath reopened this Apr 17, 2019
@zeripath
Copy link
Contributor

@Governa is the link to the come_on submodule in https://try.gitea.io/arandomer/arepoopo an example of this?

https://try.gitea.io/22/arandomer/come_on which should be https://try.gitea.io/arandomer/come_on

generated from git submodule add ssh://[email protected]:22/arandomer/come_on.git

Now looking at:

gitea/modules/git/submodule.go

Lines 32 to 77 in f6eedd4

func getRefURL(refURL, urlPrefix, parentPath string) string {
if refURL == "" {
return ""
}
url := strings.TrimSuffix(refURL, ".git")
// git://xxx/user/repo
if strings.HasPrefix(url, "git://") {
return "http://" + strings.TrimPrefix(url, "git://")
}
// http[s]://xxx/user/repo
if strings.HasPrefix(url, "http://") || strings.HasPrefix(url, "https://") {
return url
}
// Relative url prefix check (according to git submodule documentation)
if strings.HasPrefix(url, "./") || strings.HasPrefix(url, "../") {
// ...construct and return correct submodule url here...
idx := strings.Index(parentPath, "/src/")
if idx == -1 {
return url
}
return strings.TrimSuffix(urlPrefix, "/") + parentPath[:idx] + "/" + url
}
// sysuser@xxx:user/repo
i := strings.Index(url, "@")
j := strings.LastIndex(url, ":")
// Only process when i < j because git+ssh://[email protected]/npploader.git
if i > -1 && j > -1 && i < j {
// fix problem with reverse proxy works only with local server
if strings.Contains(urlPrefix, url[i+1:j]) {
return urlPrefix + url[j+1:]
}
if strings.HasPrefix(url, "ssh://") || strings.HasPrefix(url, "git+ssh://") {
k := strings.Index(url[j+1:], "/")
return "http://" + url[i+1:j] + "/" + url[j+1:][k+1:]
}
return "http://" + url[i+1:j] + "/" + url[j+1:]
}
return url
}

I'm not particularly fond of this code - Why aren't we using the default url parsing code built into go?

Strangely it seems like a similiar case is being tested for in:

gitea/modules/git/submodule_test.go

Line 23 in f6eedd4

{"ssh://[email protected]:2222/zefie/lge_g6_kernel_scripts.git", "/", "/", "http://git.zefie.net/zefie/lge_g6_kernel_scripts"},

@lunny
Copy link
Member

lunny commented Apr 26, 2019

I think [email protected]:a/b.git is not a standard URL.

@zeripath
Copy link
Contributor

zeripath commented Apr 26, 2019 •
edited
Loading

yep it isn't - just tested it.

@stale
Copy link

stale bot commented Jun 25, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 weeks. Thank you for your contributions.

@stale stale bot added the issue/stale label Jun 25, 2019
@lunny lunny added the issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented label Jun 26, 2019
@stale stale bot removed the issue/stale label Jun 26, 2019
@Governa
Copy link
Author

Governa commented Aug 15, 2019

The one case I had bookmarked is now working, so works for me 👍

@mrsdizzie
Copy link
Member

This has since been fixed and included in testcases

@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.
Assignees
No one assigned
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix submodule has port
Fixes incorrect URL for submodules that specify the remote port Governa/git
6 participants
@lunny @Governa @lafriks @mrsdizzie @zeripath @thehowl