Incorrect link for submodules that specify the remote port

[Governa]

• Gitea version (or commit ref):
  d4c9399 and any older one that I've tested
• Operating system: Ubuntu 16.04 (using the docker image)
• Database (use [x]):
  • PostgreSQL
  • MySQL
  • MSSQL
  • SQLite
• Can you reproduce the bug at https://try.gitea.io:
  • Yes (provide example URL)
    https://try.gitea.io/BugReproduction871239/test213
  • No
  • Not relevant
• Log gist:

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.

[Governa]

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

[thehowl]

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]

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]

This has been merged to latest Docker image I think.

[Governa]

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]

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

[zeripath]

@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]

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

[zeripath]

yep it isn't - just tested it.

[stale]

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.

[Governa]

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

[mrsdizzie]

This has since been fixed and included in testcases