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

Refactor submodule URL parsing #7100

Merged
merged 2 commits into from
Jun 3, 2019
Merged

Refactor submodule URL parsing #7100

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ddf1c09
Refactor submodule URL parsing
mrsdizzie Jun 1, 2019
513d946
Merge branch 'master' into fix-1526
lunny Jun 3, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 62 additions & 26 deletions modules/git/submodule.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,19 @@
// Copyright 2019 The Gitea Authors. All rights reserved.
// Copyright 2015 The Gogs Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.

package git

import "strings"
import (
"fmt"
"net"
"net/url"
"regexp"
"strings"
)

var scpSyntax = regexp.MustCompile(`^([a-zA-Z0-9_]+@)?([a-zA-Z0-9._-]+):(.*)$`)

// SubModule submodule is a reference on git repository
type SubModule struct {
Expand Down Expand Up @@ -34,46 +43,73 @@ func getRefURL(refURL, urlPrefix, parentPath string) string {
return ""
}

url := strings.TrimSuffix(refURL, ".git")

// git://xxx/user/repo
if strings.HasPrefix(url, "git://") {
return "http://" + strings.TrimPrefix(url, "git://")
}
refURI := strings.TrimSuffix(refURL, ".git")

// http[s]://xxx/user/repo
if strings.HasPrefix(url, "http://") || strings.HasPrefix(url, "https://") {
return url
prefixURL, _ := url.Parse(urlPrefix)
urlPrefixHostname, _, err := net.SplitHostPort(prefixURL.Host)
if err != nil {
urlPrefixHostname = prefixURL.Host
}

// Relative url prefix check (according to git submodule documentation)
if strings.HasPrefix(url, "./") || strings.HasPrefix(url, "../") {
if strings.HasPrefix(refURI, "./") || strings.HasPrefix(refURI, "../") {
// ...construct and return correct submodule url here...
idx := strings.Index(parentPath, "/src/")
if idx == -1 {
return url
return refURI
}
return strings.TrimSuffix(urlPrefix, "/") + parentPath[:idx] + "/" + url
return strings.TrimSuffix(urlPrefix, "/") + parentPath[:idx] + "/" + refURI
}

// sysuser@xxx:user/repo
i := strings.Index(url, "@")
j := strings.LastIndex(url, ":")
if !strings.Contains(refURI, "://") {
// scp style syntax which contains *no* port number after the : (and is not parsed by net/url)
// ex: git@try.gitea.io:go-gitea/gitea
match := scpSyntax.FindAllStringSubmatch(refURI, -1)
if len(match) > 0 {

// Only process when i < j because git+ssh://git@git.forwardbias.in/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:]
m := match[0]
refHostname := m[2]
path := m[3]

if !strings.HasPrefix(path, "/") {
path = "/" + path
}

if urlPrefixHostname == refHostname {
return prefixURL.Scheme + "://" + urlPrefixHostname + path
}
return "http://" + refHostname + path
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it default to https better?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wouldn't work if the server didn't support https (we don't know anything about it in this case). most servers should issue a redirect from http to https if they do support it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lafriks what do you think? I agree with @mrsdizzie here - most https sites will simply redirect http to https.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true but from security point of view it would be better to prefer https and as most of users should be running https this would result in unneeded http request to server.. So it is trade-off for security vs usability :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we might need to add some repository configuration to allow for people to set their own conversions for submodules, but it's not particularly high priority.

}
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:]
}

ref, err := url.Parse(refURI)
if err != nil {
return ""
}

refHostname, _, err := net.SplitHostPort(ref.Host)
if err != nil {
refHostname = ref.Host
}

supportedSchemes := []string{"http", "https", "git", "ssh", "git+ssh"}

for _, scheme := range supportedSchemes {
if ref.Scheme == scheme {
if urlPrefixHostname == refHostname {
return prefixURL.Scheme + "://" + prefixURL.Host + ref.Path
} else if ref.Scheme == "http" || ref.Scheme == "https" {
if len(ref.User.Username()) > 0 {
return ref.Scheme + "://" + fmt.Sprintf("%v", ref.User) + "@" + ref.Host + ref.Path
}
return ref.Scheme + "://" + ref.Host + ref.Path
} else {
return "http://" + refHostname + ref.Path
}
}
return "http://" + url[i+1:j] + "/" + url[j+1:]
}

return url
return ""
}

// RefURL guesses and returns reference URL.
Expand Down
13 changes: 12 additions & 1 deletion modules/git/submodule_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,19 @@ func TestGetRefURL(t *testing.T) {
}{
{"git://github.com/user1/repo1", "/", "/", "http://github.com/user1/repo1"},
{"https://localhost/user1/repo1.git", "/", "/", "https://localhost/user1/repo1"},
{"git@github.com/user1/repo1.git", "/", "/", "git@github.com/user1/repo1"},
{"http://localhost/user1/repo1.git", "/", "/", "http://localhost/user1/repo1"},
{"git@github.com:user1/repo1.git", "/", "/", "http://github.com/user1/repo1"},
{"ssh://git@git.zefie.net:2222/zefie/lge_g6_kernel_scripts.git", "/", "/", "http://git.zefie.net/zefie/lge_g6_kernel_scripts"},
{"git@git.zefie.net:2222/zefie/lge_g6_kernel_scripts.git", "/", "/", "http://git.zefie.net/2222/zefie/lge_g6_kernel_scripts"},
{"git@try.gitea.io:go-gitea/gitea", "https://try.gitea.io/go-gitea/gitea", "/", "https://try.gitea.io/go-gitea/gitea"},
{"ssh://git@try.gitea.io:9999/go-gitea/gitea", "https://try.gitea.io/go-gitea/gitea", "/", "https://try.gitea.io/go-gitea/gitea"},
{"git://git@try.gitea.io:9999/go-gitea/gitea", "https://try.gitea.io/go-gitea/log", "/", "https://try.gitea.io/go-gitea/gitea"},
{"ssh://git@127.0.0.1:9999/go-gitea/gitea", "https://127.0.0.1:3000/go-gitea/log", "/", "https://127.0.0.1:3000/go-gitea/gitea"},
{"https://gitea.com:3000/user1/repo1.git", "https://127.0.0.1:3000/go-gitea/gitea", "/", "https://gitea.com:3000/user1/repo1"},
{"https://username:password@github.com/username/repository.git", "/", "/", "https://username:password@github.com/username/repository"},
{"somethingbad", "https://127.0.0.1:3000/go-gitea/gitea", "/", ""},
{"git@localhost:user/repo", "https://localhost/user/repo2", "/", "https://localhost/user/repo"},
{"../path/to/repo.git/", "https://localhost/user/repo2/src/branch/master/test", "/", "../path/to/repo.git/"},
}

for _, kase := range kases {
Expand Down