Skip to content

Commit

Permalink
Check PR number value before casting from int64 to int (#4516)
Browse files Browse the repository at this point in the history
* Check PR number value before casting from int64 to int

This should not matter as int == int64 on 64bit architectures IIRC but
let's silence the code scanning alerts.

* Silence linter
  • Loading branch information
jhrozek authored Sep 17, 2024
1 parent 292c4ce commit f6ba405
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 3 deletions.
9 changes: 9 additions & 0 deletions internal/engine/ingester/diff/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"bufio"
"context"
"fmt"
"math"
"path/filepath"
"regexp"
"strconv"
Expand Down Expand Up @@ -78,6 +79,8 @@ func (di *Diff) GetConfig() protoreflect.ProtoMessage {
}

// Ingest ingests a diff from a pull request in accordance with its type
//
//nolint:gocyclo
func (di *Diff) Ingest(
ctx context.Context,
ent protoreflect.ProtoMessage,
Expand All @@ -99,6 +102,9 @@ func (di *Diff) Ingest(
case "", pb.DiffTypeDep:
allDiffs := make([]*pbinternal.PrDependencies_ContextualDependency, 0)
for {
if pr.Number > math.MaxInt {
return nil, fmt.Errorf("pr number is too large")
}
prFiles, resp, err := di.cli.ListFiles(ctx, pr.RepoOwner, pr.RepoName, int(pr.Number), prFilesPerPage, page)
if err != nil {
return nil, fmt.Errorf("error getting pull request files: %w", err)
Expand Down Expand Up @@ -131,6 +137,9 @@ func (di *Diff) Ingest(
case pb.DiffTypeFull:
allDiffs := make([]*pbinternal.PrContents_File, 0)
for {
if pr.Number > math.MaxInt {
return nil, fmt.Errorf("pr number is too large")
}
prFiles, resp, err := di.cli.ListFiles(ctx, pr.RepoOwner, pr.RepoName, int(pr.Number), prFilesPerPage, page)
if err != nil {
return nil, fmt.Errorf("error getting pull request files: %w", err)
Expand Down
12 changes: 9 additions & 3 deletions internal/providers/github/properties/pull_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package properties
import (
"context"
"fmt"
"math"
"net/http"
"strconv"
"strings"
Expand Down Expand Up @@ -126,12 +127,17 @@ func getPrWrapper(
) (map[string]any, error) {
_ = isOrg

owner, name, id, err := getPrWrapperAttrsFromProps(getByProps)
owner, name, id64, err := getPrWrapperAttrsFromProps(getByProps)
if err != nil {
return nil, fmt.Errorf("error getting pr wrapper attributes: %w", err)
}

prReply, result, err := ghCli.PullRequests.Get(ctx, owner, name, int(id))
if id64 > math.MaxInt {
return nil, fmt.Errorf("pr number is too large")
}
intId := int(id64)

prReply, result, err := ghCli.PullRequests.Get(ctx, owner, name, intId)
if err != nil {
if result != nil && result.StatusCode == http.StatusNotFound {
return nil, v1.ErrEntityNotFound
Expand All @@ -142,7 +148,7 @@ func getPrWrapper(
prProps := map[string]any{
// general entity
properties.PropertyUpstreamID: prReply.GetID(),
properties.PropertyName: fmt.Sprintf("%s/%s/%d", owner, name, id),
properties.PropertyName: fmt.Sprintf("%s/%s/%d", owner, name, intId),
// github-specific
PullPropertyURL: prReply.GetHTMLURL(),
// our proto representation uses int64 for the number but GH uses int
Expand Down

0 comments on commit f6ba405

Please sign in to comment.