Skip to content

Commit

Permalink
Fix issue in AcceptedError handling for UploadSarif (#3047)
Browse files Browse the repository at this point in the history
Fixes: #3036.
  • Loading branch information
Rigdon authored Jan 10, 2024
1 parent 9231a0f commit 207a38d
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 5 deletions.
18 changes: 15 additions & 3 deletions github/code-scanning.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ package github

import (
"context"
"encoding/json"
"errors"
"fmt"
"strconv"
"strings"
Expand Down Expand Up @@ -389,11 +391,21 @@ func (s *CodeScanningService) UploadSarif(ctx context.Context, owner, repo strin
return nil, nil, err
}

sarifID := new(SarifID)
resp, err := s.client.Do(ctx, req, sarifID)
if err != nil {
// This will always return an error without unmarshalling the data
resp, err := s.client.Do(ctx, req, nil)
// Even though there was an error, we still return the response
// in case the caller wants to inspect it further.
// However, if the error is AcceptedError, decode it below before
// returning from this function and closing the response body.
var acceptedError *AcceptedError
if !errors.As(err, &acceptedError) {
return nil, resp, err
}
sarifID := new(SarifID)
decErr := json.Unmarshal(acceptedError.Raw, sarifID)
if decErr != nil {
return nil, resp, decErr
}

return sarifID, resp, nil
}
Expand Down
14 changes: 12 additions & 2 deletions github/code-scanning_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ func TestCodeScanningService_UploadSarif(t *testing.T) {
client, mux, _, teardown := setup()
defer teardown()

expectedSarifID := &SarifID{
ID: String("testid"),
URL: String("https://example.com/testurl"),
}

mux.HandleFunc("/repos/o/r/code-scanning/sarifs", func(w http.ResponseWriter, r *http.Request) {
v := new(SarifAnalysis)
assertNilError(t, json.NewDecoder(r.Body).Decode(v))
Expand All @@ -67,15 +72,20 @@ func TestCodeScanningService_UploadSarif(t *testing.T) {
t.Errorf("Request body = %+v, want %+v", v, want)
}

fmt.Fprint(w, `{"commit_sha":"abc","ref":"ref/head/main","sarif":"abc"}`)
w.WriteHeader(http.StatusAccepted)
respBody, _ := json.Marshal(expectedSarifID)
_, _ = w.Write(respBody)
})

ctx := context.Background()
sarifAnalysis := &SarifAnalysis{CommitSHA: String("abc"), Ref: String("ref/head/main"), Sarif: String("abc"), CheckoutURI: String("uri"), StartedAt: &Timestamp{time.Date(2006, time.January, 02, 15, 04, 05, 0, time.UTC)}, ToolName: String("codeql-cli")}
_, _, err := client.CodeScanning.UploadSarif(ctx, "o", "r", sarifAnalysis)
respSarifID, _, err := client.CodeScanning.UploadSarif(ctx, "o", "r", sarifAnalysis)
if err != nil {
t.Errorf("CodeScanning.UploadSarif returned error: %v", err)
}
if !cmp.Equal(expectedSarifID, respSarifID) {
t.Errorf("Sarif response = %+v, want %+v", respSarifID, expectedSarifID)
}

const methodName = "UploadSarif"
testBadOptions(t, methodName, func() (err error) {
Expand Down

0 comments on commit 207a38d

Please sign in to comment.