From 231083647ef96fc97fd4dff6b88b93450a677693 Mon Sep 17 00:00:00 2001
From: Youfu Zhang <1315097+zhangyoufu@users.noreply.github.com>
Date: Fri, 19 Sep 2025 11:46:26 +0800
Subject: [PATCH] pikpak: fix unnecessary retries by using URL expire parameter
 - fixes #8601

Before this change, rclone would unnecessarily retry downloads when
the `Link.Expire` field was unreliable but the download URL contained
a valid expire query parameter. This primarily affects cases where
media links are unavailable or when `no_media_link` is enabled.

The `Link.Valid()` method now primarily checks the URL's expire query
parameter (as Unix timestamp) and falls back to the Expire field
only when URL parsing fails. This eliminates the `error no link`
retry loops while maintaining backward compatibility.

Signed-off-by: Youfu Zhang <zhangyoufu@gmail.com>
---
 backend/pikpak/api/types.go      | 20 ++++++-
 backend/pikpak/api/types_test.go | 99 ++++++++++++++++++++++++++++++++
 2 files changed, 118 insertions(+), 1 deletion(-)
 create mode 100644 backend/pikpak/api/types_test.go

diff --git a/backend/pikpak/api/types.go b/backend/pikpak/api/types.go
index 0e2d8049b..e167ffcfd 100644
--- a/backend/pikpak/api/types.go
+++ b/backend/pikpak/api/types.go
@@ -5,6 +5,7 @@ package api
 
 import (
 	"fmt"
+	"net/url"
 	"reflect"
 	"strconv"
 	"time"
@@ -136,8 +137,25 @@ type Link struct {
 }
 
 // Valid reports whether l is non-nil, has an URL, and is not expired.
+// It primarily checks the URL's expire query parameter, falling back to the Expire field.
 func (l *Link) Valid() bool {
-	return l != nil && l.URL != "" && time.Now().Add(10*time.Second).Before(time.Time(l.Expire))
+	if l == nil || l.URL == "" {
+		return false
+	}
+
+	// Primary validation: check URL's expire query parameter
+	if u, err := url.Parse(l.URL); err == nil {
+		if expireStr := u.Query().Get("expire"); expireStr != "" {
+			// Try parsing as Unix timestamp (seconds)
+			if expireInt, err := strconv.ParseInt(expireStr, 10, 64); err == nil {
+				expireTime := time.Unix(expireInt, 0)
+				return time.Now().Add(10 * time.Second).Before(expireTime)
+			}
+		}
+	}
+
+	// Fallback validation: use the Expire field if URL parsing didn't work
+	return time.Now().Add(10 * time.Second).Before(time.Time(l.Expire))
 }
 
 // URL is a basic form of URL
diff --git a/backend/pikpak/api/types_test.go b/backend/pikpak/api/types_test.go
new file mode 100644
index 000000000..d560f7572
--- /dev/null
+++ b/backend/pikpak/api/types_test.go
@@ -0,0 +1,99 @@
+package api
+
+import (
+	"fmt"
+	"testing"
+	"time"
+)
+
+// TestLinkValid tests the Link.Valid method for various scenarios
+func TestLinkValid(t *testing.T) {
+	tests := []struct {
+		name     string
+		link     *Link
+		expected bool
+		desc     string
+	}{
+		{
+			name:     "nil link",
+			link:     nil,
+			expected: false,
+			desc:     "nil link should be invalid",
+		},
+		{
+			name:     "empty URL",
+			link:     &Link{URL: ""},
+			expected: false,
+			desc:     "empty URL should be invalid",
+		},
+		{
+			name: "valid URL with future expire parameter",
+			link: &Link{
+				URL: fmt.Sprintf("https://example.com/file?expire=%d", time.Now().Add(time.Hour).Unix()),
+			},
+			expected: true,
+			desc:     "URL with future expire parameter should be valid",
+		},
+		{
+			name: "expired URL with past expire parameter",
+			link: &Link{
+				URL: fmt.Sprintf("https://example.com/file?expire=%d", time.Now().Add(-time.Hour).Unix()),
+			},
+			expected: false,
+			desc:     "URL with past expire parameter should be invalid",
+		},
+		{
+			name: "URL expire parameter takes precedence over Expire field",
+			link: &Link{
+				URL:    fmt.Sprintf("https://example.com/file?expire=%d", time.Now().Add(time.Hour).Unix()),
+				Expire: Time(time.Now().Add(-time.Hour)), // Fallback is expired
+			},
+			expected: true,
+			desc:     "URL expire parameter should take precedence over Expire field",
+		},
+		{
+			name: "URL expire parameter within 10 second buffer should be invalid",
+			link: &Link{
+				URL: fmt.Sprintf("https://example.com/file?expire=%d", time.Now().Add(5*time.Second).Unix()),
+			},
+			expected: false,
+			desc:     "URL expire parameter within 10 second buffer should be invalid",
+		},
+		{
+			name: "fallback to Expire field when no URL expire parameter",
+			link: &Link{
+				URL:    "https://example.com/file",
+				Expire: Time(time.Now().Add(time.Hour)),
+			},
+			expected: true,
+			desc:     "should fallback to Expire field when URL has no expire parameter",
+		},
+		{
+			name: "fallback to Expire field when URL expire parameter is invalid",
+			link: &Link{
+				URL:    "https://example.com/file?expire=invalid",
+				Expire: Time(time.Now().Add(time.Hour)),
+			},
+			expected: true,
+			desc:     "should fallback to Expire field when URL expire parameter is unparseable",
+		},
+		{
+			name: "invalid when both URL expire and Expire field are expired",
+			link: &Link{
+				URL:    fmt.Sprintf("https://example.com/file?expire=%d", time.Now().Add(-time.Hour).Unix()),
+				Expire: Time(time.Now().Add(-time.Hour)),
+			},
+			expected: false,
+			desc:     "should be invalid when both URL expire and Expire field are expired",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := tt.link.Valid()
+			if result != tt.expected {
+				t.Errorf("Link.Valid() = %v, expected %v. %s", result, tt.expected, tt.desc)
+			}
+		})
+	}
+}