Skip to content

Commit a66ba48

Browse files
fix panic due to defer reading body of failed request
1 parent 6a57e75 commit a66ba48

File tree

2 files changed

+44
-2
lines changed

2 files changed

+44
-2
lines changed

pkg/github/repository_resource.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,13 +138,14 @@ func RepositoryResourceContentsHandler(getClient GetClientFn, getRawClient raw.G
138138
}
139139

140140
resp, err := rawClient.GetRawContent(ctx, owner, repo, path, rawOpts)
141+
if err != nil {
142+
return nil, fmt.Errorf("failed to get raw content: %w", err)
143+
}
141144
defer func() {
142145
_ = resp.Body.Close()
143146
}()
144147
// If the raw content is not found, we will fall back to the GitHub API (in case it is a directory)
145148
switch {
146-
case err != nil:
147-
return nil, fmt.Errorf("failed to get raw content: %w", err)
148149
case resp.StatusCode == http.StatusOK:
149150
ext := filepath.Ext(path)
150151
mimeType := resp.Header.Get("Content-Type")

pkg/github/repository_resource_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package github
22

33
import (
44
"context"
5+
"errors"
56
"net/http"
67
"net/url"
78
"testing"
@@ -14,6 +15,15 @@ import (
1415
"github.com/stretchr/testify/require"
1516
)
1617

18+
// errorTransport is a http.RoundTripper that always returns an error.
19+
type errorTransport struct {
20+
err error
21+
}
22+
23+
func (t *errorTransport) RoundTrip(*http.Request) (*http.Response, error) {
24+
return nil, t.err
25+
}
26+
1727
func Test_repositoryResourceContentsHandler(t *testing.T) {
1828
base, _ := url.Parse("https://raw.example.com/")
1929
tests := []struct {
@@ -256,6 +266,37 @@ func Test_repositoryResourceContentsHandler(t *testing.T) {
256266
}
257267
}
258268

269+
// Test_repositoryResourceContentsHandler_NetworkError tests that a network error
270+
// during raw content fetch does not cause a panic (nil response body dereference).
271+
func Test_repositoryResourceContentsHandler_NetworkError(t *testing.T) {
272+
base, _ := url.Parse("https://raw.example.com/")
273+
networkErr := errors.New("network error: connection refused")
274+
275+
httpClient := &http.Client{Transport: &errorTransport{err: networkErr}}
276+
client := github.NewClient(httpClient)
277+
mockRawClient := raw.NewClient(client, base)
278+
handler := RepositoryResourceContentsHandler(stubGetClientFn(client), stubGetRawClientFn(mockRawClient))
279+
280+
request := mcp.ReadResourceRequest{
281+
Params: struct {
282+
URI string `json:"uri"`
283+
Arguments map[string]any `json:"arguments,omitempty"`
284+
}{
285+
Arguments: map[string]any{
286+
"owner": []string{"owner"},
287+
"repo": []string{"repo"},
288+
"path": []string{"README.md"},
289+
},
290+
},
291+
}
292+
293+
// This should not panic, even though the HTTP client returns an error
294+
resp, err := handler(context.TODO(), request)
295+
require.Error(t, err)
296+
require.Nil(t, resp)
297+
require.ErrorContains(t, err, "failed to get raw content")
298+
}
299+
259300
func Test_GetRepositoryResourceContent(t *testing.T) {
260301
mockRawClient := raw.NewClient(github.NewClient(nil), &url.URL{})
261302
tmpl, _ := GetRepositoryResourceContent(nil, stubGetRawClientFn(mockRawClient), translations.NullTranslationHelper)

0 commit comments

Comments
 (0)