From f9b2efdd5cf2de2f53467a958d4768c314c890ab Mon Sep 17 00:00:00 2001
From: Thom Shutt <thomshutt@users.noreply.github.com>
Date: Fri, 26 Jul 2024 16:18:26 +0100
Subject: [PATCH] Better clipping request validation (#1348)

---
 handlers/upload.go      | 49 ++++++++++++++++++++++++++---
 handlers/upload_test.go | 68 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 112 insertions(+), 5 deletions(-)

diff --git a/handlers/upload.go b/handlers/upload.go
index d8a16fe2f..a1359f465 100644
--- a/handlers/upload.go
+++ b/handlers/upload.go
@@ -108,13 +108,39 @@ func (r UploadVODRequest) IsProfileValid() bool {
 	return true
 }
 
-func (r UploadVODRequest) IsClipValid() bool {
+func (r UploadVODRequest) IsClippingRequest() bool {
+	return r.hasTargetOutput(func(o UploadVODRequestOutputLocationOutputs) string {
+		return o.Clip
+	})
+}
+
+func (r UploadVODRequest) ValidateClippingRequest() error {
 	startTime := r.ClipStrategy.StartTime
 	endTime := r.ClipStrategy.EndTime
-	if startTime < 0 || endTime <= 0 || startTime == endTime || startTime > endTime {
-		return false
+
+	if startTime < 0 {
+		return fmt.Errorf("clip start time %d cannot be less than 0", startTime)
 	}
-	return true
+	if endTime < 0 {
+		return fmt.Errorf("clip end time %d cannot be less than 0", endTime)
+	}
+
+	if startTime >= 1000000000 && startTime <= 9999999999 {
+		return fmt.Errorf("clip start time %d is in unix seconds, but should be milliseconds", startTime)
+	}
+	if endTime >= 1000000000 && endTime <= 9999999999 {
+		return fmt.Errorf("clip end time %d is in unix seconds, but should be milliseconds", endTime)
+	}
+
+	if startTime == endTime {
+		return fmt.Errorf("clip start time and end time were both %d but should be different", startTime)
+	}
+
+	if startTime > endTime {
+		return fmt.Errorf("clip start time %d should be after end time %d", startTime, endTime)
+	}
+
+	return nil
 }
 
 func (r UploadVODRequest) getTargetMp4Output() (UploadVODRequestOutputLocation, bool) {
@@ -139,6 +165,15 @@ func (r UploadVODRequest) getSourceCopyEnabled() bool {
 
 type getOutput func(UploadVODRequestOutputLocationOutputs) string
 
+func (r UploadVODRequest) hasTargetOutput(getOutput getOutput) bool {
+	for _, o := range r.OutputLocations {
+		if getOutput(o.Outputs) == "enabled" {
+			return true
+		}
+	}
+	return false
+}
+
 func (r UploadVODRequest) getTargetOutput(getOutput getOutput) UploadVODRequestOutputLocation {
 	for _, o := range r.OutputLocations {
 		if getOutput(o.Outputs) == "enabled" {
@@ -208,7 +243,11 @@ func (d *CatalystAPIHandlersCollection) handleUploadVOD(w http.ResponseWriter, r
 	// Check if this is a clipping request
 	var clipTargetURL *url.URL
 	var err error
-	if uploadVODRequest.IsClipValid() {
+	if uploadVODRequest.IsClippingRequest() {
+		if err := uploadVODRequest.ValidateClippingRequest(); err != nil {
+			return false, errors.WriteHTTPBadRequest(w, "Invalid Clipping Request", err)
+		}
+
 		clipTargetOutput := uploadVODRequest.getTargetOutput(func(o UploadVODRequestOutputLocationOutputs) string {
 			return o.Clip
 		})
diff --git a/handlers/upload_test.go b/handlers/upload_test.go
index 5f71fc765..747c9d3a0 100644
--- a/handlers/upload_test.go
+++ b/handlers/upload_test.go
@@ -197,3 +197,71 @@ func TestIsProfileValid(t *testing.T) {
 		})
 	}
 }
+
+func TestWeCanDetermineIfItsAClippingRequest(t *testing.T) {
+	u := UploadVODRequest{
+		OutputLocations: []UploadVODRequestOutputLocation{
+			{
+				URL:  "some-url",
+				Type: "clip",
+				Outputs: UploadVODRequestOutputLocationOutputs{
+					Clip: "enabled",
+				},
+			},
+		},
+	}
+	require.True(t, u.IsClippingRequest())
+
+	u = UploadVODRequest{
+		OutputLocations: []UploadVODRequestOutputLocation{
+			{
+				URL:  "some-url",
+				Type: "vod",
+				Outputs: UploadVODRequestOutputLocationOutputs{
+					HLS:        "enabled",
+					MP4:        "enabled",
+					Thumbnails: "enabled",
+				},
+			},
+		},
+	}
+	require.False(t, u.IsClippingRequest())
+}
+
+func TestWeRejectInvalidClippingRequests(t *testing.T) {
+	u := UploadVODRequest{
+		OutputLocations: []UploadVODRequestOutputLocation{
+			{
+				URL:  "some-url",
+				Type: "clip",
+				Outputs: UploadVODRequestOutputLocationOutputs{
+					Clip: "enabled",
+				},
+			},
+		},
+	}
+
+	u.ClipStrategy.StartTime = 0
+	u.ClipStrategy.EndTime = 0
+	require.EqualError(t, u.ValidateClippingRequest(), "clip start time and end time were both 0 but should be different")
+
+	u.ClipStrategy.StartTime = -1
+	u.ClipStrategy.EndTime = 0
+	require.EqualError(t, u.ValidateClippingRequest(), "clip start time -1 cannot be less than 0")
+
+	u.ClipStrategy.StartTime = 123123123123
+	u.ClipStrategy.EndTime = 1
+	require.EqualError(t, u.ValidateClippingRequest(), "clip start time 123123123123 should be after end time 1")
+
+	u.ClipStrategy.StartTime = 0
+	u.ClipStrategy.EndTime = -1
+	require.EqualError(t, u.ValidateClippingRequest(), "clip end time -1 cannot be less than 0")
+
+	u.ClipStrategy.StartTime = 1722005308
+	u.ClipStrategy.EndTime = 1722005309
+	require.EqualError(t, u.ValidateClippingRequest(), "clip start time 1722005308 is in unix seconds, but should be milliseconds")
+
+	u.ClipStrategy.StartTime = 1722005308000
+	u.ClipStrategy.EndTime = 1722005309
+	require.EqualError(t, u.ValidateClippingRequest(), "clip end time 1722005309 is in unix seconds, but should be milliseconds")
+}