Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions internal/proxy/rest_backend_caller_tool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,19 @@ func TestRestBackendCaller_ExtractOwnerRepoNumber(t *testing.T) {
wantRepo: "myrepo",
wantNumber: "7",
},
{
name: "json.Number number",
args: map[string]interface{}{
"owner": "myorg",
"repo": "myrepo",
"issue_number": json.Number("99"),
},
numberKey: "issue_number",
toolName: "issue_read",
wantOwner: "myorg",
wantRepo: "myrepo",
wantNumber: "99",
},
{
name: "missing values",
args: map[string]interface{}{
Expand All @@ -89,6 +102,61 @@ func TestRestBackendCaller_ExtractOwnerRepoNumber(t *testing.T) {
toolName: "pull_request_read",
wantErr: "pull_request_read: missing owner/repo/pullNumber",
},
{
name: "float64 non-integer returns invalid error",
args: map[string]interface{}{
"owner": "myorg",
"repo": "myrepo",
"issue_number": float64(1.5),
},
numberKey: "issue_number",
toolName: "issue_read",
wantErr: "issue_read: invalid issue_number (out of range or not an integer)",
},
{
name: "float64 negative returns invalid error",
args: map[string]interface{}{
"owner": "myorg",
"repo": "myrepo",
"issue_number": float64(-1),
},
numberKey: "issue_number",
toolName: "issue_read",
wantErr: "issue_read: invalid issue_number (must be non-negative)",
},
{
name: "float64 out-of-range returns invalid error",
args: map[string]interface{}{
"owner": "myorg",
"repo": "myrepo",
"issue_number": float64(1e20),
},
numberKey: "issue_number",
toolName: "issue_read",
wantErr: "issue_read: invalid issue_number (out of range or not an integer)",
},
{
name: "json.Number non-integer returns invalid error",
args: map[string]interface{}{
"owner": "myorg",
"repo": "myrepo",
"issue_number": json.Number("1.5"),
},
numberKey: "issue_number",
toolName: "issue_read",
wantErr: "issue_read: invalid issue_number (out of range or not an integer)",
},
{
name: "json.Number negative returns invalid error",
args: map[string]interface{}{
"owner": "myorg",
"repo": "myrepo",
"issue_number": json.Number("-1"),
},
numberKey: "issue_number",
toolName: "issue_read",
wantErr: "issue_read: invalid issue_number (must be non-negative)",
},
}

for _, tt := range tests {
Expand Down
24 changes: 13 additions & 11 deletions internal/proxy/router.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package proxy

import (
"encoding/json"
"fmt"
"regexp"
"strings"
Expand Down Expand Up @@ -77,21 +78,22 @@ func extractOwnerRepoNumber(argsMap map[string]interface{}, ownerKey, repoKey, n
repo = strutil.GetStringFromMap(argsMap, repoKey)
number = strutil.GetStringFromMap(argsMap, numberKey)
if number == "" {
if n, ok := argsMap[numberKey].(float64); ok {
logRouter.Printf("extractOwnerRepoNumber: %s provided as float64=%v, parsing as integer for tool=%s", numberKey, n, toolName)
const maxInt64AsFloat = float64(int64(^uint64(0) >> 1))
if n < 0 || n > maxInt64AsFloat {
logRouter.Printf("extractOwnerRepoNumber: %s float64=%v out of int64 range [0,%v] for tool=%s", numberKey, n, maxInt64AsFloat, toolName)
err = fmt.Errorf("%s: invalid %s (out of range)", toolName, numberKey)
rawVal := argsMap[numberKey]
switch rawVal.(type) {
case float64, json.Number:
s, ok := strutil.InterfaceToIntString(rawVal)
if !ok {
logRouter.Printf("extractOwnerRepoNumber: %s=%v is not a valid integer for tool=%s", numberKey, rawVal, toolName)
err = fmt.Errorf("%s: invalid %s (out of range or not an integer)", toolName, numberKey)
return
}
i := int64(n)
if n != float64(i) {
logRouter.Printf("extractOwnerRepoNumber: %s float64=%v is not a whole number for tool=%s", numberKey, n, toolName)
err = fmt.Errorf("%s: invalid %s (expected integer)", toolName, numberKey)
if len(s) > 0 && s[0] == '-' {
logRouter.Printf("extractOwnerRepoNumber: %s=%v is negative for tool=%s", numberKey, rawVal, toolName)
err = fmt.Errorf("%s: invalid %s (must be non-negative)", toolName, numberKey)
return
}
number = fmt.Sprintf("%d", i)
logRouter.Printf("extractOwnerRepoNumber: %s provided as numeric=%v, parsing as integer for tool=%s", numberKey, rawVal, toolName)
number = s
}
Comment on lines 80 to 97
}
if owner == "" || repo == "" || number == "" {
Expand Down
9 changes: 2 additions & 7 deletions internal/server/difc_log.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,8 @@ func buildFilteredItemLogEntry(serverID, toolName string, detail difc.FilteredIt
}
}
entry.HTMLURL = strutil.GetStringFromMap(m, "html_url", "htmlUrl")
if n, ok := m["number"]; ok {
switch v := n.(type) {
case float64:
entry.Number = fmt.Sprintf("%d", int64(v))
case json.Number:
entry.Number = v.String()
}
if s, ok := strutil.InterfaceToIntString(m["number"]); ok {
entry.Number = s
}
entry.SHA = strutil.GetStringFromMap(m, "sha")
logDifcLog.Printf("Filtered item metadata: author=%s, number=%s, url=%s", entry.AuthorLogin, entry.Number, entry.HTMLURL)
Expand Down
2 changes: 1 addition & 1 deletion internal/server/difc_log_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ func TestBuildFilteredItemLogEntry_NumberEdgeCases(t *testing.T) {
wantNumber string
}{
{name: "zero float64", data: map[string]interface{}{"number": float64(0)}, wantNumber: "0"},
{name: "decimal float64 truncates", data: map[string]interface{}{"number": float64(123.9)}, wantNumber: "123"},
{name: "decimal float64 returns empty", data: map[string]interface{}{"number": float64(123.9)}, wantNumber: ""},
{name: "nil number returns empty", data: map[string]interface{}{"number": nil}, wantNumber: ""},
{name: "int number returns empty", data: map[string]interface{}{"number": 42}, wantNumber: ""},
}
Expand Down
38 changes: 38 additions & 0 deletions internal/strutil/numbers.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package strutil

import (
"encoding/json"
"fmt"
"math"
)

// InterfaceToIntString attempts to convert a JSON-decoded numeric interface value
// (float64 or json.Number) to its decimal integer string representation.
// Returns ("", false) if the value is not a numeric type or is non-integer.
func InterfaceToIntString(v interface{}) (string, bool) {
switch n := v.(type) {
case float64:
// Explicitly guard against out-of-range values before conversion, since
// converting an out-of-range float64 to int64 is implementation-defined in Go.
// float64(math.MaxInt64) rounds up to 9.223372036854776e18, so use >=
// for the upper bound. float64(math.MinInt64) = -(2^63) is exactly
// representable, so < is appropriate for the lower bound.
if n < float64(math.MinInt64) || n >= float64(math.MaxInt64) {
return "", false // out of int64 range
}
i := int64(n)
if n != float64(i) {
return "", false // non-integer float
}
return fmt.Sprintf("%d", i), true
case json.Number:
// Validate that the json.Number represents a valid integer and convert to
// a canonical decimal string (avoids non-canonical forms like "00123").
i, err := n.Int64()
if err != nil {
return "", false // non-integer or out-of-range json.Number
}
return fmt.Sprintf("%d", i), true
}
return "", false
}
111 changes: 111 additions & 0 deletions internal/strutil/numbers_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
package strutil

import (
"encoding/json"
"testing"

"github.com/stretchr/testify/assert"
)

func TestInterfaceToIntString(t *testing.T) {
t.Parallel()

t.Run("float64 integer", func(t *testing.T) {
t.Parallel()
s, ok := InterfaceToIntString(float64(42))
assert.True(t, ok)
assert.Equal(t, "42", s)
})

t.Run("float64 zero", func(t *testing.T) {
t.Parallel()
s, ok := InterfaceToIntString(float64(0))
assert.True(t, ok)
assert.Equal(t, "0", s)
})

t.Run("float64 negative integer", func(t *testing.T) {
t.Parallel()
s, ok := InterfaceToIntString(float64(-7))
assert.True(t, ok)
assert.Equal(t, "-7", s)
})

t.Run("float64 non-integer returns false", func(t *testing.T) {
t.Parallel()
s, ok := InterfaceToIntString(float64(1.5))
assert.False(t, ok)
assert.Equal(t, "", s)
})

t.Run("float64 truncatable decimal returns false", func(t *testing.T) {
t.Parallel()
s, ok := InterfaceToIntString(float64(123.9))
assert.False(t, ok)
assert.Equal(t, "", s)
})

t.Run("float64 out of int64 range returns false", func(t *testing.T) {
t.Parallel()
// 1e20 exceeds int64 max; explicit out-of-range guard rejects it
s, ok := InterfaceToIntString(float64(1e20))
assert.False(t, ok)
assert.Equal(t, "", s)
})

t.Run("json.Number integer", func(t *testing.T) {
t.Parallel()
s, ok := InterfaceToIntString(json.Number("999"))
assert.True(t, ok)
assert.Equal(t, "999", s)
})

t.Run("json.Number leading zeros canonicalized", func(t *testing.T) {
t.Parallel()
s, ok := InterfaceToIntString(json.Number("00123"))
assert.True(t, ok)
assert.Equal(t, "123", s)
})

t.Run("json.Number large value within int64", func(t *testing.T) {
t.Parallel()
s, ok := InterfaceToIntString(json.Number("9223372036854775807"))
assert.True(t, ok)
assert.Equal(t, "9223372036854775807", s)
})

t.Run("json.Number decimal returns false", func(t *testing.T) {
t.Parallel()
s, ok := InterfaceToIntString(json.Number("123.45"))
assert.False(t, ok)
assert.Equal(t, "", s)
})

t.Run("json.Number out of int64 range returns false", func(t *testing.T) {
t.Parallel()
s, ok := InterfaceToIntString(json.Number("99999999999999999999"))
assert.False(t, ok)
assert.Equal(t, "", s)
})

t.Run("string returns false", func(t *testing.T) {
t.Parallel()
s, ok := InterfaceToIntString("42")
assert.False(t, ok)
assert.Equal(t, "", s)
})

t.Run("int returns false", func(t *testing.T) {
t.Parallel()
s, ok := InterfaceToIntString(42)
assert.False(t, ok)
assert.Equal(t, "", s)
})

t.Run("nil returns false", func(t *testing.T) {
t.Parallel()
s, ok := InterfaceToIntString(nil)
assert.False(t, ok)
assert.Equal(t, "", s)
})
}
Loading