Handle Contents Edge-Case (#492)

close #459

add Tests for #485

Reviewed-on: https://gitea.com/gitea/go-sdk/pulls/492
Reviewed-by: Andrew Thornton <art27@cantab.net>
Reviewed-by: khmarbaise <khmarbaise@noreply.gitea.io>
Co-authored-by: 6543 <6543@obermui.de>
Co-committed-by: 6543 <6543@obermui.de>

gitea/repo_file.go

@@ -131,6 +131,7 @@ func pathEscapeSegments(path string) string {
 func (c *Client) GetFile(owner, repo, ref, filepath string) ([]byte, *Response, error) {
 	filepath = pathEscapeSegments(filepath)
 	if c.checkServerVersionGreaterThanOrEqual(version1_14_0) != nil {
+		ref = pathEscapeSegments(ref)
 		return c.getResponse("GET", fmt.Sprintf("/repos/%s/%s/raw/%s/%s", owner, repo, ref, filepath), nil, nil)
 	}
 	return c.getResponse("GET", fmt.Sprintf("/repos/%s/%s/raw/%s?ref=%s", owner, repo, filepath, url.QueryEscape(ref)), nil, nil)
@@ -139,21 +140,34 @@ func (c *Client) GetFile(owner, repo, ref, filepath string) ([]byte, *Response,
 // GetContents get the metadata and contents of a file in a repository
 // ref is optional
 func (c *Client) GetContents(owner, repo, ref, filepath string) (*ContentsResponse, *Response, error) {
-	filepath = pathEscapeSegments(filepath)
+	data, resp, err := c.getDirOrFileContents(owner, repo, ref, filepath)
+	if err != nil {
+		return nil, resp, err
+	}
 	cr := new(ContentsResponse)
-	filepath = strings.TrimPrefix(filepath, "/")
+	if json.Unmarshal(data, &cr) != nil {
-	resp, err := c.getParsedResponse("GET", fmt.Sprintf("/repos/%s/%s/contents/%s?ref=%s", owner, repo, filepath, url.QueryEscape(ref)), jsonHeader, nil, cr)
+		return nil, resp, fmt.Errorf("expect file, got directory")
+	}
 	return cr, resp, err
 }
 
 // ListContents gets a list of entries in a dir
 // ref is optional
 func (c *Client) ListContents(owner, repo, ref, filepath string) ([]*ContentsResponse, *Response, error) {
-	filepath = pathEscapeSegments(filepath)
+	data, resp, err := c.getDirOrFileContents(owner, repo, ref, filepath)
-	cr := make([]*ContentsResponse, 0)
+	if err != nil {
-	filepath = strings.TrimPrefix(filepath, "/")
+		return nil, resp, err
-	resp, err := c.getParsedResponse("GET", fmt.Sprintf("/repos/%s/%s/contents/%s?ref=%s", owner, repo, filepath, url.QueryEscape(ref)), jsonHeader, nil, &cr)
+	}
-	return cr, resp, err
+	crl := make([]*ContentsResponse, 0)
+	if json.Unmarshal(data, &crl) != nil {
+		return nil, resp, fmt.Errorf("expect directory, got file")
+	}
+	return crl, resp, err
+}
+
+func (c *Client) getDirOrFileContents(owner, repo, ref, filepath string) ([]byte, *Response, error) {
+	filepath = pathEscapeSegments(strings.TrimPrefix(filepath, "/"))
+	return c.getResponse("GET", fmt.Sprintf("/repos/%s/%s/contents/%s?ref=%s", owner, repo, filepath, url.QueryEscape(ref)), jsonHeader, nil)
 }
 
 // CreateFile create a file in a repository

gitea/repo_file_test.go

@@ -59,7 +59,7 @@ func TestFileCreateUpdateGet(t *testing.T) {
 	})
 	assert.NoError(t, err)
 	_, resp, err := c.GetFile(repo.Owner.UserName, repo.Name, "master", testFileName)
-	assert.EqualValues(t, "404 Not Found", err.Error())
+	assert.Error(t, err)
 	assert.EqualValues(t, 404, resp.StatusCode)
 
 	licence, _, err := c.GetContents(repo.Owner.UserName, repo.Name, "", "LICENSE")
@@ -82,4 +82,35 @@ func TestFileCreateUpdateGet(t *testing.T) {
 	assert.NotNil(t, licence)
 	assert.False(t, bytes.Equal(licenceRaw, licenceRawNew))
 	assert.EqualValues(t, testContent, base64.StdEncoding.EncodeToString(licenceRawNew))
+
+	// ListContents in root dir of default branch
+	dir, resp, err := c.ListContents(repo.Owner.UserName, repo.Name, "", "")
+	assert.NoError(t, err)
+	assert.Len(t, dir, 3)
+	assert.NotNil(t, resp)
+
+	// ListContents in not existing dir of default branch
+	_, resp, err = c.ListContents(repo.Owner.UserName, repo.Name, "", "/hehe/")
+	assert.Error(t, err)
+	assert.EqualValues(t, 404, resp.StatusCode)
+	// ListContents in root dir of not existing branch
+	_, resp, err = c.ListContents(repo.Owner.UserName, repo.Name, "no-ref-at-all", "")
+	assert.Error(t, err)
+	assert.EqualValues(t, 404, resp.StatusCode)
+
+	// ListContents try to get file as dir
+	dir, resp, err = c.ListContents(repo.Owner.UserName, repo.Name, "", "LICENSE")
+	if assert.Error(t, err) {
+		assert.EqualValues(t, "expect directory, got file", err.Error())
+	}
+	assert.Nil(t, dir)
+	assert.EqualValues(t, 200, resp.StatusCode)
+
+	// GetContents try to get dir as file
+	file, resp, err = c.GetContents(repo.Owner.UserName, repo.Name, "", "")
+	if assert.Error(t, err) {
+		assert.EqualValues(t, "expect file, got directory", err.Error())
+	}
+	assert.Nil(t, file)
+	assert.EqualValues(t, 200, resp.StatusCode)
 }