From 9035c418302f1c8c1a3af26d54e011b5ca37e5aa Mon Sep 17 00:00:00 2001 From: Wayne Starr Date: Mon, 7 Nov 2022 14:26:15 -0600 Subject: [PATCH 1/5] Simplify tests and refactor name/version checks --- routers/api/packages/pypi/pypi.go | 12 ++++--- routers/api/packages/pypi/pypi_test.go | 36 +++++++++++++++++++++ tests/integration/api_packages_pypi_test.go | 6 ++-- 3 files changed, 47 insertions(+), 7 deletions(-) create mode 100644 routers/api/packages/pypi/pypi_test.go diff --git a/routers/api/packages/pypi/pypi.go b/routers/api/packages/pypi/pypi.go index 66380d832cef..311617647689 100644 --- a/routers/api/packages/pypi/pypi.go +++ b/routers/api/packages/pypi/pypi.go @@ -21,9 +21,9 @@ import ( packages_service "code.gitea.io/gitea/services/packages" ) -// https://www.python.org/dev/peps/pep-0503/#normalized-names +// https://peps.python.org/pep-0426/#name var normalizer = strings.NewReplacer(".", "-", "_", "-") -var nameMatcher = regexp.MustCompile(`\A[a-zA-Z0-9\.\-_]+\z`) +var nameMatcher = regexp.MustCompile(`\A(?:[a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9._-]*[a-zA-Z0-9])\z`) // https://peps.python.org/pep-0440/#appendix-b-parsing-version-strings-with-regular-expressions var versionMatcher = regexp.MustCompile(`\Av?` + @@ -128,7 +128,7 @@ func UploadPackageFile(ctx *context.Context) { packageName := normalizer.Replace(ctx.Req.FormValue("name")) packageVersion := ctx.Req.FormValue("version") - if !nameMatcher.MatchString(packageName) || !versionMatcher.MatchString(packageVersion) { + if hasInvalidMetadata(packageName, packageVersion) { apiError(ctx, http.StatusBadRequest, "invalid name or version") return } @@ -146,7 +146,7 @@ func UploadPackageFile(ctx *context.Context) { Name: packageName, Version: packageVersion, }, - SemverCompatible: true, + SemverCompatible: false, Creator: ctx.Doer, Metadata: &pypi_module.Metadata{ Author: ctx.Req.FormValue("author"), @@ -177,3 +177,7 @@ func UploadPackageFile(ctx *context.Context) { ctx.Status(http.StatusCreated) } + +func hasInvalidMetadata(packageName, packageVersion string) bool { + return !nameMatcher.MatchString(packageName) || !versionMatcher.MatchString(packageVersion) +} diff --git a/routers/api/packages/pypi/pypi_test.go b/routers/api/packages/pypi/pypi_test.go new file mode 100644 index 000000000000..84fcac5c2170 --- /dev/null +++ b/routers/api/packages/pypi/pypi_test.go @@ -0,0 +1,36 @@ +// Copyright 2022 The Gitea 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 pypi + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestHasInvalidMetadata(t *testing.T) { + // The test cases below were created from the following Python PEPs: + // https://peps.python.org/pep-0426/#name + // https://peps.python.org/pep-0440/#appendix-b-parsing-version-strings-with-regular-expressions + + // Valid Cases + assert.False(t, hasInvalidMetadata("Test.Name.1234", "1.0.1")) + assert.False(t, hasInvalidMetadata("test_name", "1.0.1")) + assert.False(t, hasInvalidMetadata("test-name", "1.0.1")) + assert.False(t, hasInvalidMetadata("test-name", "v1.0.1")) + assert.False(t, hasInvalidMetadata("test-name", "2012.4")) + assert.False(t, hasInvalidMetadata("test-name", "1.0.1-alpha")) + assert.False(t, hasInvalidMetadata("test-name", "1.0.1a1")) + assert.False(t, hasInvalidMetadata("test-name", "1.0b2.r345.dev456")) + assert.False(t, hasInvalidMetadata("test-name", "1!1.0.1")) + assert.False(t, hasInvalidMetadata("test-name", "1.0.1+local.1")) + + // Invalid Cases + assert.True(t, hasInvalidMetadata(".test-name", "1.0.1")) + assert.True(t, hasInvalidMetadata("test!name", "1.0.1")) + assert.True(t, hasInvalidMetadata("test-name", "a1.0.1")) + assert.True(t, hasInvalidMetadata("test-name", "1.0.1aa")) + assert.True(t, hasInvalidMetadata("test-name", "1.0.0-alpha.beta")) +} diff --git a/tests/integration/api_packages_pypi_test.go b/tests/integration/api_packages_pypi_test.go index 0cd6ff7d13dc..83719dcca0f4 100644 --- a/tests/integration/api_packages_pypi_test.go +++ b/tests/integration/api_packages_pypi_test.go @@ -29,7 +29,7 @@ func TestPackagePyPI(t *testing.T) { user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) packageName := "test-package" - packageVersion := "1.0.1+r1234" + packageVersion := "1!1.0.1+r1234" packageAuthor := "KN4CK3R" packageDescription := "Test Description" @@ -72,7 +72,7 @@ func TestPackagePyPI(t *testing.T) { pd, err := packages.GetPackageDescriptor(db.DefaultContext, pvs[0]) assert.NoError(t, err) - assert.NotNil(t, pd.SemVer) + assert.Nil(t, pd.SemVer) assert.IsType(t, &pypi.Metadata{}, pd.Metadata) assert.Equal(t, packageName, pd.Package.Name) assert.Equal(t, packageVersion, pd.Version.Version) @@ -100,7 +100,7 @@ func TestPackagePyPI(t *testing.T) { pd, err := packages.GetPackageDescriptor(db.DefaultContext, pvs[0]) assert.NoError(t, err) - assert.NotNil(t, pd.SemVer) + assert.Nil(t, pd.SemVer) assert.IsType(t, &pypi.Metadata{}, pd.Metadata) assert.Equal(t, packageName, pd.Package.Name) assert.Equal(t, packageVersion, pd.Version.Version) From c607b9335bfccba8e8e2434dca128e4a074fd357 Mon Sep 17 00:00:00 2001 From: Wayne Starr Date: Mon, 7 Nov 2022 14:30:42 -0600 Subject: [PATCH 2/5] Expressly escape . and - in regex --- routers/api/packages/pypi/pypi.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/api/packages/pypi/pypi.go b/routers/api/packages/pypi/pypi.go index 311617647689..15709ec9c017 100644 --- a/routers/api/packages/pypi/pypi.go +++ b/routers/api/packages/pypi/pypi.go @@ -23,7 +23,7 @@ import ( // https://peps.python.org/pep-0426/#name var normalizer = strings.NewReplacer(".", "-", "_", "-") -var nameMatcher = regexp.MustCompile(`\A(?:[a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9._-]*[a-zA-Z0-9])\z`) +var nameMatcher = regexp.MustCompile(`\A(?:[a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\.\-_]*[a-zA-Z0-9])\z`) // https://peps.python.org/pep-0440/#appendix-b-parsing-version-strings-with-regular-expressions var versionMatcher = regexp.MustCompile(`\Av?` + From 5f730ca4b4f732730c1cd2146ef1c72206fa76a5 Mon Sep 17 00:00:00 2001 From: Wayne Starr Date: Mon, 7 Nov 2022 14:34:57 -0600 Subject: [PATCH 3/5] Add another test case --- routers/api/packages/pypi/pypi_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/routers/api/packages/pypi/pypi_test.go b/routers/api/packages/pypi/pypi_test.go index 84fcac5c2170..458ad4d6cd65 100644 --- a/routers/api/packages/pypi/pypi_test.go +++ b/routers/api/packages/pypi/pypi_test.go @@ -16,6 +16,7 @@ func TestHasInvalidMetadata(t *testing.T) { // https://peps.python.org/pep-0440/#appendix-b-parsing-version-strings-with-regular-expressions // Valid Cases + assert.False(t, hasInvalidMetadata("A", "1.0.1")) assert.False(t, hasInvalidMetadata("Test.Name.1234", "1.0.1")) assert.False(t, hasInvalidMetadata("test_name", "1.0.1")) assert.False(t, hasInvalidMetadata("test-name", "1.0.1")) From 3242b2c304530fc2d8abdb8ab20382f09c6ba19c Mon Sep 17 00:00:00 2001 From: Wayne Starr Date: Mon, 7 Nov 2022 15:34:58 -0600 Subject: [PATCH 4/5] Flip boolean in name+version check --- routers/api/packages/pypi/pypi.go | 6 ++--- routers/api/packages/pypi/pypi_test.go | 34 +++++++++++++------------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/routers/api/packages/pypi/pypi.go b/routers/api/packages/pypi/pypi.go index 15709ec9c017..4c8041c30cc4 100644 --- a/routers/api/packages/pypi/pypi.go +++ b/routers/api/packages/pypi/pypi.go @@ -128,7 +128,7 @@ func UploadPackageFile(ctx *context.Context) { packageName := normalizer.Replace(ctx.Req.FormValue("name")) packageVersion := ctx.Req.FormValue("version") - if hasInvalidMetadata(packageName, packageVersion) { + if !isValidNameAndVersion(packageName, packageVersion) { apiError(ctx, http.StatusBadRequest, "invalid name or version") return } @@ -178,6 +178,6 @@ func UploadPackageFile(ctx *context.Context) { ctx.Status(http.StatusCreated) } -func hasInvalidMetadata(packageName, packageVersion string) bool { - return !nameMatcher.MatchString(packageName) || !versionMatcher.MatchString(packageVersion) +func isValidNameAndVersion(packageName, packageVersion string) bool { + return nameMatcher.MatchString(packageName) && versionMatcher.MatchString(packageVersion) } diff --git a/routers/api/packages/pypi/pypi_test.go b/routers/api/packages/pypi/pypi_test.go index 458ad4d6cd65..d66e4088afb4 100644 --- a/routers/api/packages/pypi/pypi_test.go +++ b/routers/api/packages/pypi/pypi_test.go @@ -10,28 +10,28 @@ import ( "github.com/stretchr/testify/assert" ) -func TestHasInvalidMetadata(t *testing.T) { +func TestIsValidNameAndVersion(t *testing.T) { // The test cases below were created from the following Python PEPs: // https://peps.python.org/pep-0426/#name // https://peps.python.org/pep-0440/#appendix-b-parsing-version-strings-with-regular-expressions // Valid Cases - assert.False(t, hasInvalidMetadata("A", "1.0.1")) - assert.False(t, hasInvalidMetadata("Test.Name.1234", "1.0.1")) - assert.False(t, hasInvalidMetadata("test_name", "1.0.1")) - assert.False(t, hasInvalidMetadata("test-name", "1.0.1")) - assert.False(t, hasInvalidMetadata("test-name", "v1.0.1")) - assert.False(t, hasInvalidMetadata("test-name", "2012.4")) - assert.False(t, hasInvalidMetadata("test-name", "1.0.1-alpha")) - assert.False(t, hasInvalidMetadata("test-name", "1.0.1a1")) - assert.False(t, hasInvalidMetadata("test-name", "1.0b2.r345.dev456")) - assert.False(t, hasInvalidMetadata("test-name", "1!1.0.1")) - assert.False(t, hasInvalidMetadata("test-name", "1.0.1+local.1")) + assert.True(t, isValidNameAndVersion("A", "1.0.1")) + assert.True(t, isValidNameAndVersion("Test.Name.1234", "1.0.1")) + assert.True(t, isValidNameAndVersion("test_name", "1.0.1")) + assert.True(t, isValidNameAndVersion("test-name", "1.0.1")) + assert.True(t, isValidNameAndVersion("test-name", "v1.0.1")) + assert.True(t, isValidNameAndVersion("test-name", "2012.4")) + assert.True(t, isValidNameAndVersion("test-name", "1.0.1-alpha")) + assert.True(t, isValidNameAndVersion("test-name", "1.0.1a1")) + assert.True(t, isValidNameAndVersion("test-name", "1.0b2.r345.dev456")) + assert.True(t, isValidNameAndVersion("test-name", "1!1.0.1")) + assert.True(t, isValidNameAndVersion("test-name", "1.0.1+local.1")) // Invalid Cases - assert.True(t, hasInvalidMetadata(".test-name", "1.0.1")) - assert.True(t, hasInvalidMetadata("test!name", "1.0.1")) - assert.True(t, hasInvalidMetadata("test-name", "a1.0.1")) - assert.True(t, hasInvalidMetadata("test-name", "1.0.1aa")) - assert.True(t, hasInvalidMetadata("test-name", "1.0.0-alpha.beta")) + assert.False(t, isValidNameAndVersion(".test-name", "1.0.1")) + assert.False(t, isValidNameAndVersion("test!name", "1.0.1")) + assert.False(t, isValidNameAndVersion("test-name", "a1.0.1")) + assert.False(t, isValidNameAndVersion("test-name", "1.0.1aa")) + assert.False(t, isValidNameAndVersion("test-name", "1.0.0-alpha.beta")) } From c9372be15ccb316290fad8929ec8f82cc744eb0e Mon Sep 17 00:00:00 2001 From: Wayne Starr Date: Mon, 7 Nov 2022 15:49:38 -0600 Subject: [PATCH 5/5] Update routers/api/packages/pypi/pypi_test.go Co-authored-by: KN4CK3R --- routers/api/packages/pypi/pypi_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/routers/api/packages/pypi/pypi_test.go b/routers/api/packages/pypi/pypi_test.go index d66e4088afb4..56e327a3472a 100644 --- a/routers/api/packages/pypi/pypi_test.go +++ b/routers/api/packages/pypi/pypi_test.go @@ -31,6 +31,8 @@ func TestIsValidNameAndVersion(t *testing.T) { // Invalid Cases assert.False(t, isValidNameAndVersion(".test-name", "1.0.1")) assert.False(t, isValidNameAndVersion("test!name", "1.0.1")) + assert.False(t, isValidNameAndVersion("-test-name", "1.0.1")) + assert.False(t, isValidNameAndVersion("test-name-", "1.0.1")) assert.False(t, isValidNameAndVersion("test-name", "a1.0.1")) assert.False(t, isValidNameAndVersion("test-name", "1.0.1aa")) assert.False(t, isValidNameAndVersion("test-name", "1.0.0-alpha.beta"))