fix: review findings — NaN threshold guard, percent-escape rejection in upstream model ids
- Segment: reject NaN thresholds (NaN fails every comparison, so it passed the [0,1] range check and reached the shim as the literal string "NaN"); ±Inf were already caught by the range comparisons, now covered by tests too. - upstreamPath: reject '%' in model ids — %2F/%2E%2E percent-escapes decode back into path structure server-side, bypassing the literal /?#/.. rejection. Ids never legitimately contain '%'. Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01WWCQcYStWXBYUy5sZWnbLT
This commit is contained in:
@@ -238,7 +238,7 @@ func TestInterpolateRejectsBadArgs(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func TestUpstreamPathRejectsSeparators(t *testing.T) {
|
func TestUpstreamPathRejectsSeparators(t *testing.T) {
|
||||||
for _, bad := range []string{"", "a/b", "a?b", "a#b"} {
|
for _, bad := range []string{"", "a/b", "a?b", "a#b", "a%2Fb", "%2e%2e", "a%b"} {
|
||||||
if _, err := upstreamPath(bad, "/x"); err == nil {
|
if _, err := upstreamPath(bad, "/x"); err == nil {
|
||||||
t.Errorf("upstreamPath(%q) succeeded; want error", bad)
|
t.Errorf("upstreamPath(%q) succeeded; want error", bad)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -14,6 +14,7 @@ package llamaswap
|
|||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"math"
|
||||||
"net/http"
|
"net/http"
|
||||||
"strconv"
|
"strconv"
|
||||||
|
|
||||||
@@ -45,7 +46,9 @@ func (m *segmentationModel) Segment(ctx context.Context, req imagegen.Segmentati
|
|||||||
if req.Prompt == "" {
|
if req.Prompt == "" {
|
||||||
return nil, fmt.Errorf("%w: segmentation requires a prompt", llm.ErrUnsupported)
|
return nil, fmt.Errorf("%w: segmentation requires a prompt", llm.ErrUnsupported)
|
||||||
}
|
}
|
||||||
if req.Threshold < 0 || req.Threshold > 1 {
|
// NaN fails every comparison, so it would sail through a bare range
|
||||||
|
// check and reach the upstream as the literal string "NaN".
|
||||||
|
if math.IsNaN(req.Threshold) || req.Threshold < 0 || req.Threshold > 1 {
|
||||||
return nil, fmt.Errorf("%w: segmentation threshold must be in [0,1], got %g", llm.ErrUnsupported, req.Threshold)
|
return nil, fmt.Errorf("%w: segmentation threshold must be in [0,1], got %g", llm.ErrUnsupported, req.Threshold)
|
||||||
}
|
}
|
||||||
path, err := upstreamPath(m.id, "/v1/segment")
|
path, err := upstreamPath(m.id, "/v1/segment")
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ package llamaswap
|
|||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"errors"
|
"errors"
|
||||||
|
"math"
|
||||||
"net/http"
|
"net/http"
|
||||||
"net/http/httptest"
|
"net/http/httptest"
|
||||||
"testing"
|
"testing"
|
||||||
@@ -95,6 +96,16 @@ func TestSegmentRejectsBadArgs(t *testing.T) {
|
|||||||
imagegen.SegmentationRequest{Image: imagegen.Image{Data: []byte{1}}, Prompt: "dog", Threshold: 1.5}); !errors.Is(err, llm.ErrUnsupported) {
|
imagegen.SegmentationRequest{Image: imagegen.Image{Data: []byte{1}}, Prompt: "dog", Threshold: 1.5}); !errors.Is(err, llm.ErrUnsupported) {
|
||||||
t.Errorf("threshold 1.5: err = %v, want ErrUnsupported", err)
|
t.Errorf("threshold 1.5: err = %v, want ErrUnsupported", err)
|
||||||
}
|
}
|
||||||
|
for name, bad := range map[string]float64{
|
||||||
|
"NaN": math.NaN(),
|
||||||
|
"+Inf": math.Inf(1),
|
||||||
|
"-Inf": math.Inf(-1),
|
||||||
|
} {
|
||||||
|
if _, err := sg.Segment(context.Background(),
|
||||||
|
imagegen.SegmentationRequest{Image: imagegen.Image{Data: []byte{1}}, Prompt: "dog", Threshold: bad}); !errors.Is(err, llm.ErrUnsupported) {
|
||||||
|
t.Errorf("threshold %s: err = %v, want ErrUnsupported", name, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestSegmentRejectsNonImageResponse(t *testing.T) {
|
func TestSegmentRejectsNonImageResponse(t *testing.T) {
|
||||||
|
|||||||
@@ -16,12 +16,14 @@ import (
|
|||||||
//
|
//
|
||||||
// Why reject rather than escape: same rationale as Unload — model ids
|
// Why reject rather than escape: same rationale as Unload — model ids
|
||||||
// legitimately contain ":" but never path-structure characters, and escaping
|
// legitimately contain ":" but never path-structure characters, and escaping
|
||||||
// would mask a config error instead of surfacing it.
|
// would mask a config error instead of surfacing it. '%' is rejected too:
|
||||||
|
// ids never legitimately carry percent-escapes, and %2F/%2E%2E would decode
|
||||||
|
// back into path structure on the server side.
|
||||||
func upstreamPath(model, rest string) (string, error) {
|
func upstreamPath(model, rest string) (string, error) {
|
||||||
if strings.TrimSpace(model) == "" {
|
if strings.TrimSpace(model) == "" {
|
||||||
return "", fmt.Errorf("llama-swap: upstream call requires a model id")
|
return "", fmt.Errorf("llama-swap: upstream call requires a model id")
|
||||||
}
|
}
|
||||||
if strings.ContainsAny(model, "/?#") || strings.Contains(model, "..") {
|
if strings.ContainsAny(model, "/?#%") || strings.Contains(model, "..") {
|
||||||
return "", fmt.Errorf("llama-swap: invalid model id %q for upstream call (contains a path separator)", model)
|
return "", fmt.Errorf("llama-swap: invalid model id %q for upstream call (contains a path separator)", model)
|
||||||
}
|
}
|
||||||
if !strings.HasPrefix(rest, "/") {
|
if !strings.HasPrefix(rest, "/") {
|
||||||
|
|||||||
Reference in New Issue
Block a user