Skip to content

Commit eef0d24

Browse files
committed
fix: escape jobsink location path segments
Signed-off-by: immanuwell <pchpr.00@list.ru>
1 parent f5180bc commit eef0d24

2 files changed

Lines changed: 90 additions & 12 deletions

File tree

cmd/jobsink/main.go

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"fmt"
2525
"log"
2626
"net/http"
27+
"net/url"
2728
"strings"
2829
"time"
2930

@@ -424,18 +425,13 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
424425

425426
func (h *Handler) handleGet(ctx context.Context, w http.ResponseWriter, r *http.Request) {
426427
logger := logging.FromContext(ctx)
427-
parts := strings.Split(strings.TrimSuffix(r.RequestURI, "/"), "/")
428-
if len(parts) != 9 {
428+
ref, eventSource, eventID, err := parseLocation(r.RequestURI)
429+
if err != nil {
429430
logger.Info("Malformed uri", zap.String("URI", r.RequestURI))
430431
w.WriteHeader(http.StatusBadRequest)
431432
return
432433
}
433434

434-
ref := types.NamespacedName{
435-
Namespace: parts[2],
436-
Name: parts[4],
437-
}
438-
439435
js, err := h.lister.JobSinks(ref.Namespace).Get(ref.Name)
440436
if err != nil {
441437
logger.Warn("Failed to retrieve jobsink", zap.String("ref", ref.String()), zap.Error(err))
@@ -450,10 +446,6 @@ func (h *Handler) handleGet(ctx context.Context, w http.ResponseWriter, r *http.
450446
logger.Warn("Failed to verify AuthN and AuthZ.", zap.Error(err))
451447
return
452448
}
453-
454-
eventSource := parts[6]
455-
eventID := parts[8]
456-
457449
jobName := toJobName(ref.Name, eventSource, eventID)
458450

459451
job, err := h.k8s.BatchV1().Jobs(ref.Namespace).Get(r.Context(), jobName, metav1.GetOptions{})
@@ -499,13 +491,45 @@ func getServerTLSConfig(ctx context.Context) (*tls.Config, error) {
499491
}
500492

501493
func locationHeader(ref types.NamespacedName, source, id string) string {
502-
return fmt.Sprintf("/namespaces/%s/name/%s/sources/%s/ids/%s", ref.Namespace, ref.Name, source, id)
494+
return fmt.Sprintf(
495+
"/namespaces/%s/name/%s/sources/%s/ids/%s",
496+
url.PathEscape(ref.Namespace),
497+
url.PathEscape(ref.Name),
498+
url.PathEscape(source),
499+
url.PathEscape(id),
500+
)
503501
}
504502

505503
func jobLabelSelector(ref types.NamespacedName, id string) string {
506504
return fmt.Sprintf("%s=%s,%s=%s", sinks.JobSinkIDLabel, id, sinks.JobSinkNameLabel, ref.Name)
507505
}
508506

507+
func parseLocation(requestURI string) (types.NamespacedName, string, string, error) {
508+
parts := strings.Split(strings.TrimSuffix(requestURI, "/"), "/")
509+
if len(parts) != 9 {
510+
return types.NamespacedName{}, "", "", fmt.Errorf("unexpected path format")
511+
}
512+
513+
namespace, err := url.PathUnescape(parts[2])
514+
if err != nil {
515+
return types.NamespacedName{}, "", "", fmt.Errorf("invalid namespace path segment: %w", err)
516+
}
517+
name, err := url.PathUnescape(parts[4])
518+
if err != nil {
519+
return types.NamespacedName{}, "", "", fmt.Errorf("invalid name path segment: %w", err)
520+
}
521+
source, err := url.PathUnescape(parts[6])
522+
if err != nil {
523+
return types.NamespacedName{}, "", "", fmt.Errorf("invalid source path segment: %w", err)
524+
}
525+
id, err := url.PathUnescape(parts[8])
526+
if err != nil {
527+
return types.NamespacedName{}, "", "", fmt.Errorf("invalid id path segment: %w", err)
528+
}
529+
530+
return types.NamespacedName{Namespace: namespace, Name: name}, source, id, nil
531+
}
532+
509533
func toJobName(js string, source, id string) string {
510534
h := md5.Sum([]byte(source + id)) //nolint:gosec
511535
return kmeta.ChildName(js+"-", utils.ToDNS1123Subdomain(hex.EncodeToString(h[:])))

cmd/jobsink/main_test.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"testing"
2121

2222
"k8s.io/apimachinery/pkg/api/validation"
23+
"k8s.io/apimachinery/pkg/types"
2324

2425
"knative.dev/eventing/pkg/utils"
2526
)
@@ -104,3 +105,56 @@ func FuzzToJobName(f *testing.F) {
104105
}
105106
})
106107
}
108+
109+
func TestLocationHeaderRoundTrip(t *testing.T) {
110+
testCases := map[string]struct {
111+
ref types.NamespacedName
112+
source string
113+
id string
114+
}{
115+
"simple": {
116+
ref: types.NamespacedName{
117+
Namespace: "test-namespace",
118+
Name: "job-sink",
119+
},
120+
source: "mysource3",
121+
id: "2234-5678",
122+
},
123+
"slashes in source and id": {
124+
ref: types.NamespacedName{
125+
Namespace: "test-namespace",
126+
Name: "job-sink",
127+
},
128+
source: "https://example.com/sources/my/source",
129+
id: "event/id/with/slashes",
130+
},
131+
}
132+
133+
for name, tc := range testCases {
134+
t.Run(name, func(t *testing.T) {
135+
location := locationHeader(tc.ref, tc.source, tc.id)
136+
137+
gotRef, gotSource, gotID, err := parseLocation(location)
138+
if err != nil {
139+
t.Fatal("unexpected error:", err)
140+
}
141+
142+
if gotRef != tc.ref {
143+
t.Fatalf("unexpected ref: got %#v want %#v", gotRef, tc.ref)
144+
}
145+
if gotSource != tc.source {
146+
t.Fatalf("unexpected source: got %q want %q", gotSource, tc.source)
147+
}
148+
if gotID != tc.id {
149+
t.Fatalf("unexpected id: got %q want %q", gotID, tc.id)
150+
}
151+
})
152+
}
153+
}
154+
155+
func TestParseLocationRejectsUnescapedSourceSlashes(t *testing.T) {
156+
_, _, _, err := parseLocation("/namespaces/test-namespace/name/job-sink/sources/https://example.com/source/ids/event-id")
157+
if err == nil {
158+
t.Fatal("expected error for malformed location")
159+
}
160+
}

0 commit comments

Comments
 (0)