1
0
mirror of https://github.com/arduino/library-registry.git synced 2025-07-07 14:41:10 +03:00

Add check for submission URL being a Git repository

With the current submission system, it's common for people to provide the tag/release URL rather than the repository
URL. I think it likely the same sort of thing will occur with the new system. Previously, a very naive check was done
for the path component of the URL to have two levels. It turns out that the Gitlab repository URLs have more levels than
that. The superior approach is to use `git ls-remote` to verify that the URL is a Git repository.
This commit is contained in:
per1234
2021-01-19 14:26:52 -08:00
parent 3fc5f76339
commit 0ca9ab8ee7
2 changed files with 23 additions and 33 deletions

View File

@ -227,11 +227,7 @@ func populateSubmission(submissionURL string, listPath *paths.Path) (submissionT
} }
// Resolve redirects and normalize. // Resolve redirects and normalize.
normalizedURLObject, err := normalizeURL(httpResponse.Request.URL) normalizedURLObject := normalizeURL(httpResponse.Request.URL)
if err != nil {
submission.Error = err.Error()
return submission, ""
}
submission.NormalizedURL = normalizedURLObject.String() submission.NormalizedURL = normalizedURLObject.String()
@ -241,6 +237,17 @@ func populateSubmission(submissionURL string, listPath *paths.Path) (submissionT
return submission, "" return submission, ""
} }
// Check if URL is a Git repository
err = exec.Command("git", "ls-remote", normalizedURLObject.String()).Run()
if err != nil {
if _, ok := err.(*exec.ExitError); ok {
submission.Error = "Submission URL is not a Git clone URL (e.g., https://github.com/arduino-libraries/Servo)."
return submission, ""
}
panic(err)
}
// Check if the URL is already in the index. // Check if the URL is already in the index.
listLines, err := listPath.ReadFileAsLines() listLines, err := listPath.ReadFileAsLines()
occurrences := 0 occurrences := 0
@ -249,11 +256,8 @@ func populateSubmission(submissionURL string, listPath *paths.Path) (submissionT
if err != nil { if err != nil {
panic(err) // All list items have already passed parsing so something is broken if this happens. panic(err) // All list items have already passed parsing so something is broken if this happens.
} }
normalizedListURLObject, err := normalizeURL(listURLObject)
if err != nil {
panic(fmt.Errorf("When processing list item %s: %s", listURL, err)) // All list items have already been through normalization without error so something is broken if this happens.
}
normalizedListURLObject := normalizeURL(listURLObject)
if normalizedListURLObject.String() == normalizedURLObject.String() { if normalizedListURLObject.String() == normalizedURLObject.String() {
occurrences++ occurrences++
if occurrences > 1 { if occurrences > 1 {
@ -350,21 +354,17 @@ func populateSubmission(submissionURL string, listPath *paths.Path) (submissionT
} }
// normalizeURL converts the URL into the standardized format used in the index. // normalizeURL converts the URL into the standardized format used in the index.
func normalizeURL(rawURL *url.URL) (url.URL, error) { func normalizeURL(rawURL *url.URL) url.URL {
normalizedPath := strings.TrimRight(rawURL.Path, "/") normalizedPath := strings.TrimRight(rawURL.Path, "/")
if !strings.HasSuffix(normalizedPath, ".git") { if !strings.HasSuffix(normalizedPath, ".git") {
normalizedPath += ".git" normalizedPath += ".git"
} }
if len(strings.Split(normalizedPath, "/")) != 3 {
return url.URL{}, fmt.Errorf("Submission URL must point to the root of the repository")
}
return url.URL{ return url.URL{
Scheme: "https", Scheme: "https",
Host: rawURL.Host, Host: rawURL.Host,
Path: normalizedPath, Path: normalizedPath,
}, }
nil
} }
func uRLIsUnder(childURL url.URL, parentCandidates []string) bool { func uRLIsUnder(childURL url.URL, parentCandidates []string) bool {

View File

@ -13,7 +13,6 @@
package main package main
import ( import (
"fmt"
"net/url" "net/url"
"testing" "testing"
@ -154,14 +153,11 @@ func Test_normalizeURL(t *testing.T) {
testName string testName string
rawURL string rawURL string
expectedNormalizedURL string expectedNormalizedURL string
expectedError string
}{ }{
{"Trailing slash", "https://github.com/foo/bar/", "https://github.com/foo/bar.git", ""}, {"Trailing slash", "https://github.com/foo/bar/", "https://github.com/foo/bar.git"},
{".git suffix", "https://github.com/foo/bar.git", "https://github.com/foo/bar.git", ""}, {".git suffix", "https://github.com/foo/bar.git", "https://github.com/foo/bar.git"},
{"Too few path elements", "https://github.com/foo", "", "Submission URL must point to the root of the repository"}, {"http://", "http://github.com/foo/bar", "https://github.com/foo/bar.git"},
{"Too many path elements", "https://github.com/foo/bar/baz", "", "Submission URL must point to the root of the repository"}, {"git://", "git://github.com/foo/bar", "https://github.com/foo/bar.git"},
{"http://", "http://github.com/foo/bar", "https://github.com/foo/bar.git", ""},
{"git://", "git://github.com/foo/bar", "https://github.com/foo/bar.git", ""},
} }
for _, testTable := range testTables { for _, testTable := range testTables {
@ -170,13 +166,7 @@ func Test_normalizeURL(t *testing.T) {
expectedNormalizedURL, err := url.Parse(testTable.expectedNormalizedURL) expectedNormalizedURL, err := url.Parse(testTable.expectedNormalizedURL)
require.Nil(t, err) require.Nil(t, err)
normalizedURL, err := normalizeURL(rawURL) assert.Equal(t, *expectedNormalizedURL, normalizeURL(rawURL), testTable.testName)
assert.Equal(t, *expectedNormalizedURL, normalizedURL, testTable.testName)
if testTable.expectedError == "" {
assert.Nil(t, err, testTable.testName)
} else {
assert.Equal(t, fmt.Errorf(testTable.expectedError), err, testTable.testName)
}
} }
} }