Verified Commit 49ef9f53 authored by Giannis Kepas's avatar Giannis Kepas Committed by GitLab
Browse files

fix: handle pkg registry filename slashes 

Changelog: Improvements
parent 50ef4942

generic_packages.go

+11 −10
Original line number Diff line number Diff line
@@ -80,17 +80,25 @@ type GenericPackagesFileURL struct { 


// FormatPackageURL returns the GitLab Package Registry URL for the given artifact metadata, without the BaseURL.

// This does not make a GitLab API request, but rather computes it based on their documentation.

//

// The file name may describe a directory structure, so its "/" characters are

// kept as path separators. File names with empty, "." or ".." segments are

// rejected with ErrInvalidFileName.

func (s *GenericPackagesService) FormatPackageURL(pid any, packageName, packageVersion, fileName string) (string, error) {

	project, err := parseID(pid)

	if err != nil {

		return "", err

	}

	escapedFileName, err := PathEscapeFileName(fileName)

	if err != nil {

		return "", err

	}

	u := fmt.Sprintf(

		"projects/%s/packages/generic/%s/%s/%s",

		PathEscape(project),

		PathEscape(packageName),

		PathEscape(packageVersion),

		PathEscape(fileName),

		escapedFileName,

	)

	return u, nil

}
@@ -110,17 +118,10 @@ type PublishPackageFileOptions struct { 
// GitLab docs:

// https://docs.gitlab.com/user/packages/generic_packages/#publish-a-single-file

func (s *GenericPackagesService) PublishPackageFile(pid any, packageName, packageVersion, fileName string, content io.Reader, opt *PublishPackageFileOptions, options ...RequestOptionFunc) (*GenericPackagesFile, *Response, error) {

	project, err := parseID(pid)

	u, err := s.FormatPackageURL(pid, packageName, packageVersion, fileName)

	if err != nil {

		return nil, nil, err

	}

	u := fmt.Sprintf(

		"projects/%s/packages/generic/%s/%s/%s",

		PathEscape(project),

		PathEscape(packageName),

		PathEscape(packageVersion),

		PathEscape(fileName),

	)



	// We need to create the request as a GET request to make sure the options

	// are set correctly. After the request is created we will overwrite both
@@ -149,7 +150,7 @@ func (s *GenericPackagesService) PublishPackageFile(pid any, packageName, packag 
// https://docs.gitlab.com/user/packages/generic_packages/#download-a-single-file

func (s *GenericPackagesService) DownloadPackageFile(pid any, packageName, packageVersion, fileName string, options ...RequestOptionFunc) ([]byte, *Response, error) {

	buf, resp, err := do[bytes.Buffer](s.client,

		withPath("projects/%s/packages/generic/%s/%s/%s", ProjectID{pid}, packageName, packageVersion, fileName),

		withPath("projects/%s/packages/generic/%s/%s/%s", ProjectID{pid}, packageName, packageVersion, FileName{fileName}),

		withRequestOpts(options...),

	)

	if err != nil {

generic_packages_test.go

+122 −3
Original line number Diff line number Diff line
@@ -43,6 +43,27 @@ func TestPublishPackageFile(t *testing.T) { 
	require.NoError(t, err)

}



func TestPublishPackageFile_DirectoryStructure(t *testing.T) {

	t.Parallel()

	// GIVEN a package file endpoint for a file name containing a directory structure

	mux, client := setup(t)



	mux.HandleFunc("/api/v4/projects/1234/packages/generic/foo/0.1.2/dir/sub.dir/bar-baz.txt", func(w http.ResponseWriter, r *http.Request) {

		testMethod(t, r, http.MethodPut)

		fmt.Fprint(w, `

       {

          "message": "201 Created"

       }

    `)

	})



	// WHEN publishing a file with "/" characters in its file name

	_, _, err := client.GenericPackages.PublishPackageFile(1234, "foo", "0.1.2", "dir/sub.dir/bar-baz.txt", strings.NewReader("bar = baz"), &PublishPackageFileOptions{})



	// THEN the "/" characters are kept as path separators and the request succeeds

	require.NoError(t, err)

}



func TestDownloadPackageFile(t *testing.T) {

	t.Parallel()

	mux, client := setup(t)
@@ -61,13 +82,111 @@ func TestDownloadPackageFile(t *testing.T) { 
	assert.Equal(t, want, packageBytes)

}



func TestDownloadPackageFile_DirectoryStructure(t *testing.T) {

	t.Parallel()

	// GIVEN a package file endpoint for a file name containing a directory structure

	mux, client := setup(t)



	mux.HandleFunc("/api/v4/projects/1234/packages/generic/foo/0.1.2/dir/sub.dir/bar-baz.txt", func(w http.ResponseWriter, r *http.Request) {

		testMethod(t, r, http.MethodGet)

		fmt.Fprint(w, strings.TrimSpace(`

       bar = baz

    `))

	})



	// WHEN downloading a file with "/" characters in its file name

	packageBytes, _, err := client.GenericPackages.DownloadPackageFile(1234, "foo", "0.1.2", "dir/sub.dir/bar-baz.txt")



	// THEN the "/" characters are kept as path separators and the file is returned

	require.NoError(t, err)

	assert.Equal(t, []byte("bar = baz"), packageBytes)

}



func TestFormatPackageURL(t *testing.T) {

	t.Parallel()



	tests := map[string]struct {

		pid      any

		fileName string

		want     string

		wantErr  bool

	}{

		"plain file name with a project ID": {

			pid:      1234,

			fileName: "bar-baz.txt",

			want:     "projects/1234/packages/generic/foo/0%2E1%2E2/bar-baz%2Etxt",

		},

		"plain file name with a namespaced project path": {

			pid:      "group/sub.group/my-project",

			fileName: "bar-baz.txt",

			want:     "projects/group%2Fsub%2Egroup%2Fmy-project/packages/generic/foo/0%2E1%2E2/bar-baz%2Etxt",

		},

		"file name maintaining a directory structure": {

			pid:      1234,

			fileName: "dir/sub dir/bar-baz.txt",

			want:     "projects/1234/packages/generic/foo/0%2E1%2E2/dir/sub%20dir/bar-baz%2Etxt",

		},

		"file name with a parent directory segment": {

			pid:      1234,

			fileName: "dir/../bar-baz.txt",

			wantErr:  true,

		},

		"file name with a leading separator": {

			pid:      1234,

			fileName: "/bar-baz.txt",

			wantErr:  true,

		},

	}



	for name, tc := range tests {

		t.Run(name, func(t *testing.T) {

			t.Parallel()

			// GIVEN a client

			_, client := setup(t)



	url, err := client.GenericPackages.FormatPackageURL(1234, "foo", "0.1.2", "bar-baz.txt")

			// WHEN formatting the package URL

			url, err := client.GenericPackages.FormatPackageURL(tc.pid, "foo", "0.1.2", tc.fileName)



			// THEN invalid file names are rejected and for valid ones only the

			// file name segments are escaped

			if tc.wantErr {

				require.ErrorIs(t, err, ErrInvalidFileName)

				assert.Empty(t, url)

				return

			}



			require.NoError(t, err)

			assert.Equal(t, tc.want, url)

		})

	}

}



func TestPublishPackageFile_InvalidFileName(t *testing.T) {

	t.Parallel()

	// GIVEN a client

	mux, client := setup(t)

	mux.HandleFunc("/", func(_ http.ResponseWriter, _ *http.Request) {

		assert.Fail(t, "no request should be sent for an invalid file name")

	})



	// WHEN publishing a file whose name contains a parent directory segment

	_, _, err := client.GenericPackages.PublishPackageFile(1234, "foo", "0.1.2", "dir/../bar-baz.txt", strings.NewReader("bar = baz"), &PublishPackageFileOptions{})



	// THEN the file name is rejected before a request is made

	require.ErrorIs(t, err, ErrInvalidFileName)

}



func TestDownloadPackageFile_InvalidFileName(t *testing.T) {

	t.Parallel()

	// GIVEN a client

	mux, client := setup(t)

	mux.HandleFunc("/", func(_ http.ResponseWriter, _ *http.Request) {

		assert.Fail(t, "no request should be sent for an invalid file name")

	})



	// WHEN downloading a file whose name contains a parent directory segment

	_, _, err := client.GenericPackages.DownloadPackageFile(1234, "foo", "0.1.2", "dir/../bar-baz.txt")



	want := "projects/1234/packages/generic/foo/0%2E1%2E2/bar-baz%2Etxt"

	assert.Equal(t, want, url)

	// THEN the file name is rejected before a request is made

	require.ErrorIs(t, err, ErrInvalidFileName)

}

gitlab.go

+23 −0
Original line number Diff line number Diff line
@@ -1335,6 +1335,29 @@ func PathEscape(s string) string { 
	return strings.ReplaceAll(url.PathEscape(s), ".", "%2E")

}



// ErrInvalidFileName is returned when a file name that is allowed to describe

// a directory structure contains a segment that can't be used in an API path.

var ErrInvalidFileName = errors.New(`the file name must not contain empty, "." or ".." segments`)



// PathEscapeFileName is a helper function to escape a file name that is

// allowed to describe a directory structure, like in the generic packages

// API. Contrary to PathEscape, the "/" characters are kept as path separators

// and only the segments in between them are escaped.

//

// Segments which would produce a malformed path are rejected with

// ErrInvalidFileName.

func PathEscapeFileName(s string) (string, error) {

	segments := strings.Split(s, "/")

	for i, segment := range segments {

		switch segment {

		case "", ".", "..":

			return "", fmt.Errorf("invalid file name %q, %w", s, ErrInvalidFileName)

		}

		segments[i] = PathEscape(segment)

	}

	return strings.Join(segments, "/"), nil

}



// An ErrorResponse reports one or more errors caused by an API request.

//

// GitLab API docs:

gitlab_test.go

+43 −0
Original line number Diff line number Diff line
@@ -337,6 +337,49 @@ func TestPathEscape(t *testing.T) { 
	assert.Equal(t, want, got)

}



func TestPathEscapeFileName(t *testing.T) {

	t.Parallel()



	tests := map[string]struct {

		fileName string

		want     string

		wantErr  bool

	}{

		"no directory structure":   {fileName: "bar-baz.txt", want: "bar-baz%2Etxt"},

		"directory structure":      {fileName: "dir/bar-baz.txt", want: "dir/bar-baz%2Etxt"},

		"segments needing escapes": {fileName: "my dir/sub#dir/bar baz.txt", want: "my%20dir/sub%23dir/bar%20baz%2Etxt"},

		"dot in a segment":         {fileName: "dir/.hidden.txt", want: "dir/%2Ehidden%2Etxt"},

		"leading separator":        {fileName: "/bar-baz.txt", wantErr: true},

		"trailing separator":       {fileName: "dir/", wantErr: true},

		"empty segment":            {fileName: "dir//bar-baz.txt", wantErr: true},

		"current directory":        {fileName: "dir/./bar-baz.txt", wantErr: true},

		"parent directory":         {fileName: "dir/../bar-baz.txt", wantErr: true},

		"only a parent directory":  {fileName: "..", wantErr: true},

		"empty":                    {fileName: "", wantErr: true},

	}



	for name, tc := range tests {

		t.Run(name, func(t *testing.T) {

			t.Parallel()

			// GIVEN a file name that may describe a directory structure

			// WHEN escaping it for use in a path

			got, err := PathEscapeFileName(tc.fileName)



			// THEN traversal and empty segments are rejected, while for valid

			// file names the "/" separators are preserved and the segments are

			// escaped

			if tc.wantErr {

				require.ErrorIs(t, err, ErrInvalidFileName)

				assert.Empty(t, got)

				return

			}



			require.NoError(t, err)

			assert.Equal(t, tc.want, got)

		})

	}

}



func TestPaginationPopulatePageValuesEmpty(t *testing.T) {

	t.Parallel()

	wantPageHeaders := map[string]int64{

request_handler.go

+14 −0
Original line number Diff line number Diff line
@@ -82,6 +82,20 @@ func (i LabelID) forPath() (string, error) { 
	return PathEscape(id), nil

}



// FileName represents a file name in an API path for endpoints that allow the

// file name to describe a directory structure, like the generic packages API.

// It is escaped with PathEscapeFileName, so the "/" characters are kept as

// path separators and only the segments in between them are escaped.

//

// GitLab API docs: https://docs.gitlab.com/user/packages/generic_packages/

type FileName struct {

	Value string

}



func (f FileName) forPath() (string, error) {

	return PathEscapeFileName(f.Value)

}



type NoEscape struct {

	Value string

}