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.

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

plumbing: Do not swallow http message coming from VCS providers #835

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 16 additions & 2 deletions plumbing/transport/http/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -406,14 +406,28 @@ func (a *TokenAuth) String() string {
// Err is a dedicated error to return errors based on status code
type Err struct {
Response *http.Response
Reason string
}

// NewErr returns a new Err based on a http response
// NewErr returns a new Err based on a http response and closes response body
// if needed
func NewErr(r *http.Response) error {
if r.StatusCode >= http.StatusOK && r.StatusCode < http.StatusMultipleChoices {
return nil
}

var reason string

// If a response message is present, add it to error
var messageBuffer bytes.Buffer
if r.Body != nil {
messageLength, _ := messageBuffer.ReadFrom(r.Body)
if messageLength > 0 {
reason = messageBuffer.String()
}
_ = r.Body.Close()
}

switch r.StatusCode {
case http.StatusUnauthorized:
return transport.ErrAuthenticationRequired
Expand All @@ -423,7 +437,7 @@ func NewErr(r *http.Response) error {
return transport.ErrRepositoryNotFound
}

return plumbing.NewUnexpectedError(&Err{r})
return plumbing.NewUnexpectedError(&Err{r, reason})
}

// StatusCode returns the status code of the response
Expand Down
19 changes: 19 additions & 0 deletions plumbing/transport/http/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package http
import (
"crypto/tls"
"fmt"
"io"
"log"
"net"
"net/http"
Expand All @@ -14,6 +15,7 @@ import (
"strings"
"testing"

"github.com/go-git/go-git/v5/plumbing"
"github.com/go-git/go-git/v5/plumbing/transport"

fixtures "github.com/go-git/go-git-fixtures/v4"
Expand Down Expand Up @@ -90,6 +92,23 @@ func (s *ClientSuite) TestNewHTTPError40x(c *C) {
"unexpected client error.*")
}

func (s *ClientSuite) TestNewUnexpectedError(c *C) {
res := &http.Response{
StatusCode: 500,
Body: io.NopCloser(strings.NewReader("Unexpected error")),
}

err := NewErr(res)
c.Assert(err, NotNil)
c.Assert(err, FitsTypeOf, &plumbing.UnexpectedError{})

unexpectedError, _ := err.(*plumbing.UnexpectedError)
c.Assert(unexpectedError.Err, FitsTypeOf, &Err{})

httpError, _ := unexpectedError.Err.(*Err)
c.Assert(httpError.Reason, Equals, "Unexpected error")
}

func (s *ClientSuite) Test_newSession(c *C) {
cl := NewClientWithOptions(nil, &ClientOptions{
CacheMaxEntries: 2,
Expand Down
1 change: 0 additions & 1 deletion plumbing/transport/http/receive_pack.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ func (s *rpSession) doRequest(
}

if err := NewErr(res); err != nil {
_ = res.Body.Close()
return nil, err
}

Expand Down
1 change: 0 additions & 1 deletion plumbing/transport/http/upload_pack.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ func (s *upSession) doRequest(
}

if err := NewErr(res); err != nil {
_ = res.Body.Close()
return nil, err
}

Expand Down