From 65b3fb10aaba62034af6ac5d3a328a09c1dc184c Mon Sep 17 00:00:00 2001 From: marcomarcogd <35049765+marcomarcogd@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:24:48 +0800 Subject: [PATCH 1/2] =?UTF-8?q?fix=EF=BC=9A=E5=9C=A8=E5=8F=91=E5=B8=83?= =?UTF-8?q?=E5=88=86=E6=94=AF=E6=81=A2=E5=A4=8D=20#1231=20=E5=93=8D?= =?UTF-8?q?=E5=BA=94=E5=BC=8F=E5=9B=BE=E7=89=87=E4=BF=AE=E5=A4=8D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- internal/handlers/core/full_text.go | 9 + internal/handlers/core/full_text_test.go | 30 ++++ .../handlers/media/media_handlers_test.go | 52 ++++++ internal/handlers/media/media_proxy.go | 156 ++++++++++-------- 4 files changed, 177 insertions(+), 70 deletions(-) diff --git a/internal/handlers/core/full_text.go b/internal/handlers/core/full_text.go index ad7463a33..17e5efb88 100644 --- a/internal/handlers/core/full_text.go +++ b/internal/handlers/core/full_text.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "fmt" + "html" "io" "net/http" "net/url" @@ -120,7 +121,9 @@ func (h *Handler) FetchFullArticleContentContext(ctx context.Context, articleURL } extracted, extractErr := readability.FromReader(strings.NewReader(page), base) var output bytes.Buffer + leadImageURL := "" if extractErr == nil { + leadImageURL = extracted.ImageURL() extractErr = extracted.RenderHTML(&output) } content := output.String() @@ -128,6 +131,12 @@ func (h *Handler) FetchFullArticleContentContext(ctx context.Context, articleURL // Explicit semantic article containers are a useful fallback for short pages. content, _ = doc.Find("article,main,[role=main]").First().Html() } + if !strings.Contains(strings.ToLower(content), "

` + content + } + } content = textutil.PrepareArticleContent(content, base.String()) if strings.TrimSpace(content) == "" { return "", fmt.Errorf("no readable article content") diff --git a/internal/handlers/core/full_text_test.go b/internal/handlers/core/full_text_test.go index 61284af1c..b57be4f3b 100644 --- a/internal/handlers/core/full_text_test.go +++ b/internal/handlers/core/full_text_test.go @@ -85,6 +85,36 @@ func TestFullTextReadabilityAndInvalidResponses(t *testing.T) { } } +func TestFullTextReadabilityRestoresLeadImageWithoutDuplicates(t *testing.T) { + h := fullTextHandler(t) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/html; charset=utf-8") + body := strings.Repeat("A long article sentence, with useful detail and punctuation. ", 30) + if r.URL.Path == "/existing" { + fmt.Fprintf(w, `

Article

%s

`, body) + return + } + fmt.Fprintf(w, `

Article

%s

`, body) + })) + defer server.Close() + + content, err := h.FetchFullArticleContentContext(context.Background(), server.URL+"/lead", nil) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(content, `src="`+server.URL+`/lead.jpg"`) || !strings.Contains(content, `referrerpolicy="no-referrer"`) { + t.Fatalf("lead image was not restored safely: %s", content) + } + + content, err = h.FetchFullArticleContentContext(context.Background(), server.URL+"/existing", nil) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(content, server.URL+"/body.jpg") || strings.Contains(content, server.URL+"/og.jpg") { + t.Fatalf("existing article image was duplicated or replaced: %s", content) + } +} + func TestFullTextNoSelectorMatchIsAnError(t *testing.T) { h := fullTextHandler(t) id, err := h.DB.AddFeed(&models.Feed{Title: "f", URL: "https://example.org"}) diff --git a/internal/handlers/media/media_handlers_test.go b/internal/handlers/media/media_handlers_test.go index afbeb0280..672317217 100644 --- a/internal/handlers/media/media_handlers_test.go +++ b/internal/handlers/media/media_handlers_test.go @@ -1,8 +1,11 @@ package media import ( + "encoding/base64" "net/http" "net/http/httptest" + "regexp" + "strings" "testing" "MrRSS/internal/database" @@ -177,6 +180,55 @@ func TestProxyImagesInHTML_RelativeURLs(t *testing.T) { } } +func TestRewriteHTMLContent_ResponsiveImageCandidates(t *testing.T) { + baseURL := "https://example.com/news/article" + htmlContent := ` + + +` + + result := string(rewriteHTMLContent([]byte(htmlContent), baseURL)) + for _, descriptor := range []string{" 1x", " 2x", " 320w", " 1280w"} { + if !strings.Contains(result, descriptor) { + t.Errorf("missing srcset descriptor %q in %s", descriptor, result) + } + } + if strings.Contains(result, `srcSet="/_next/image`) || strings.Contains(result, `data-srcset="images/`) { + t.Fatalf("responsive image candidates were not proxied: %s", result) + } + + encodedURLs := regexp.MustCompile(`url_b64=([A-Za-z0-9+/=]+)`).FindAllStringSubmatch(result, -1) + var decodedURLs []string + for _, match := range encodedURLs { + decoded, err := base64.StdEncoding.DecodeString(match[1]) + if err != nil { + t.Fatalf("decode proxied URL: %v", err) + } + decodedURLs = append(decodedURLs, string(decoded)) + } + joinedURLs := strings.Join(decodedURLs, "\n") + for _, expected := range []string{ + "https://example.com/_next/image?url=%2Fhero.jpg&w=1280&q=75", + "https://cdn.example.com/hero.jpg", + "https://example.com/news/images/small.jpg", + "https://example.com/news/images/large.jpg", + } { + if !strings.Contains(joinedURLs, expected) { + t.Errorf("missing decoded candidate %q in %s", expected, joinedURLs) + } + } + if strings.Contains(joinedURLs, "&") { + t.Fatalf("HTML entities leaked into proxied URLs: %s", joinedURLs) + } +} + +func TestRewriteSrcsetAttribute_SkipsNonHTTPAndProxiedCandidates(t *testing.T) { + content := `` + if got := rewriteSrcsetAttribute(content, "img", "srcset", "https://example.com/article"); got != content { + t.Fatalf("special srcset candidates changed:\nwant: %s\n got: %s", content, got) + } +} + func contains(s, substr string) bool { return len(s) >= len(substr) && (s == substr || len(substr) == 0 || (len(s) > 0 && findInString(s, substr))) diff --git a/internal/handlers/media/media_proxy.go b/internal/handlers/media/media_proxy.go index ec5a6c3f2..b001e666d 100644 --- a/internal/handlers/media/media_proxy.go +++ b/internal/handlers/media/media_proxy.go @@ -823,6 +823,8 @@ func rewriteHTMLContent(bodyBytes []byte, baseURL string) []byte { // Then rewrite img src attributes (now including the converted lazy images) content = rewriteAttribute(content, "img", "src", baseURL) + content = rewriteSrcsetAttribute(content, "img", "srcset", baseURL) + content = rewriteSrcsetAttribute(content, "img", "data-srcset", baseURL) // Rewrite iframe src attributes content = rewriteAttribute(content, "iframe", "src", baseURL) @@ -836,6 +838,8 @@ func rewriteHTMLContent(bodyBytes []byte, baseURL string) []byte { // Rewrite source src attributes (for video/audio) content = rewriteAttribute(content, "source", "src", baseURL) + content = rewriteSrcsetAttribute(content, "source", "srcset", baseURL) + content = rewriteSrcsetAttribute(content, "source", "data-srcset", baseURL) // Rewrite track src attributes content = rewriteAttribute(content, "track", "src", baseURL) @@ -1055,89 +1059,101 @@ func parseHTMLAttributes(tag string) []htmlAttribute { return attrs } -// rewriteAttribute rewrites a specific attribute in HTML tags +// rewriteAttribute rewrites a specific URL attribute in HTML tags. func rewriteAttribute(content, tag, attr, baseURL string) string { - // Match all tags first - tagRe := regexp.MustCompile(fmt.Sprintf(`<%s[^>]*>`, tag)) - - matchCount := 0 - rewriteCount := 0 - - result := tagRe.ReplaceAllStringFunc(content, func(match string) string { - matchCount++ - // Try to find the attribute with double quotes - doubleQuoteRe := regexp.MustCompile(fmt.Sprintf(`\s%s\s*=\s*"([^"]*)"`, attr)) - doubleQuoteMatch := doubleQuoteRe.FindStringSubmatch(match) + return rewriteAttributeValue(content, tag, attr, func(value string) (string, bool) { + return proxyWebpageResourceURL(value, baseURL) + }) +} - var urlValue, quote string +// rewriteSrcsetAttribute proxies every candidate URL while preserving its +// density or width descriptor (for example, 2x or 640w). +func rewriteSrcsetAttribute(content, tag, attr, baseURL string) string { + return rewriteAttributeValue(content, tag, attr, func(value string) (string, bool) { + var result strings.Builder + changed := false + for position := 0; position < len(value); { + prefixStart := position + for position < len(value) && (isHTMLSpace(value[position]) || value[position] == ',') { + position++ + } + result.WriteString(value[prefixStart:position]) + if position >= len(value) { + break + } - if len(doubleQuoteMatch) >= 2 { - // Found double-quoted attribute - urlValue = doubleQuoteMatch[1] - quote = `"` - } else { - // Try single quotes - singleQuoteRe := regexp.MustCompile(fmt.Sprintf(`\s%s\s*=\s*'([^']*)'`, attr)) - singleQuoteMatch := singleQuoteRe.FindStringSubmatch(match) - if len(singleQuoteMatch) >= 2 { - urlValue = singleQuoteMatch[1] - quote = `'` + urlStart := position + isDataURL := strings.HasPrefix(strings.ToLower(value[position:]), "data:") + for position < len(value) && !isHTMLSpace(value[position]) && (isDataURL || value[position] != ',') { + position++ + } + candidate := value[urlStart:position] + if proxied, ok := proxyWebpageResourceURL(candidate, baseURL); ok { + result.WriteString(proxied) + changed = true } else { - // Try unquoted - unquotedRe := regexp.MustCompile(fmt.Sprintf(`\s%s\s*=\s*([^\s>]+)`, attr)) - unquotedMatch := unquotedRe.FindStringSubmatch(match) - if len(unquotedMatch) >= 2 { - urlValue = unquotedMatch[1] - quote = "" - } else { - // Attribute not found - return match - } + result.WriteString(candidate) } - } - // Skip data: URLs, blob: URLs, and already proxied URLs - if strings.HasPrefix(urlValue, "data:") || - strings.HasPrefix(urlValue, "blob:") || - strings.HasPrefix(urlValue, "/api/") || - strings.HasPrefix(urlValue, "#") { - return match + descriptorStart := position + for position < len(value) && value[position] != ',' { + position++ + } + result.WriteString(value[descriptorStart:position]) } + return result.String(), changed + }) +} - rewriteCount++ - // if tag == "script" || tag == "link" { - // log.Printf("[%s Rewrite] Rewriting %s %d: %s", strings.ToUpper(tag), attr, rewriteCount, urlValue) - // } - - // Resolve relative URLs - resolvedURL := resolveURL(urlValue, baseURL) - - // Create proxied URL with base64 encoding - proxiedURL := fmt.Sprintf("/api/webpage/resource?url_b64=%s&referer_b64=%s", - base64.StdEncoding.EncodeToString([]byte(resolvedURL)), - base64.StdEncoding.EncodeToString([]byte(baseURL))) +func rewriteAttributeValue(content, tag, attr string, rewrite func(string) (string, bool)) string { + tagRe := regexp.MustCompile(`(?i)<` + regexp.QuoteMeta(tag) + `\b[^>]*>`) + patterns := []*regexp.Regexp{ + regexp.MustCompile(`(?i)\s+` + regexp.QuoteMeta(attr) + `\s*=\s*"([^"]*)"`), + regexp.MustCompile(`(?i)\s+` + regexp.QuoteMeta(attr) + `\s*=\s*'([^']*)'`), + regexp.MustCompile(`(?i)\s+` + regexp.QuoteMeta(attr) + `\s*=\s*([^\s>]+)`), + } - // Replace the URL in the match - // Use regex to replace attribute value more reliably - if quote != "" { - // Quoted value - replace using regex for more flexibility - attrPattern := regexp.MustCompile(`(` + attr + `)\s*=\s*` + regexp.QuoteMeta(quote) + regexp.QuoteMeta(urlValue) + regexp.QuoteMeta(quote)) - replacement := fmt.Sprintf(`%s=%s%s%s`, attr, quote, proxiedURL, quote) - return attrPattern.ReplaceAllString(match, replacement) - } else { - // Unquoted value - match until whitespace or > character - // We need to capture the delimiter (space or >) to preserve it - attrPattern := regexp.MustCompile(`(` + attr + `)\s*=\s*` + regexp.QuoteMeta(urlValue) + `([\s>])`) - replacement := fmt.Sprintf(`%s="%s"$2`, attr, proxiedURL) - return attrPattern.ReplaceAllString(match, replacement) + return tagRe.ReplaceAllStringFunc(content, func(match string) string { + for index, pattern := range patterns { + location := pattern.FindStringSubmatchIndex(match) + if len(location) < 4 { + continue + } + valueStart, valueEnd := location[2], location[3] + rewritten, changed := rewrite(match[valueStart:valueEnd]) + if !changed { + return match + } + if index == len(patterns)-1 { + rewritten = `"` + rewritten + `"` + } + return match[:valueStart] + rewritten + match[valueEnd:] } + return match }) +} - // if matchCount > 0 && (tag == "script" || tag == "link") { - // log.Printf("[%s Rewrite] Found %d %s tags, rewrote %d %s attributes", strings.ToUpper(tag), matchCount, tag, rewriteCount, attr) - // } +func proxyWebpageResourceURL(value, baseURL string) (string, bool) { + value = strings.TrimSpace(html.UnescapeString(value)) + lowerValue := strings.ToLower(value) + if value == "" || strings.HasPrefix(lowerValue, "data:") || + strings.HasPrefix(lowerValue, "blob:") || strings.HasPrefix(value, "#") || + strings.HasPrefix(value, "/api/") || strings.Contains(value, "/api/webpage/resource?") { + return value, false + } - return result + resolvedURL := resolveURL(value, baseURL) + parsedURL, err := url.Parse(resolvedURL) + if err != nil || (parsedURL.Scheme != "http" && parsedURL.Scheme != "https") { + return value, false + } + return fmt.Sprintf("/api/webpage/resource?url_b64=%s&referer_b64=%s", + base64.StdEncoding.EncodeToString([]byte(resolvedURL)), + base64.StdEncoding.EncodeToString([]byte(baseURL))), true +} + +func isHTMLSpace(value byte) bool { + return value == ' ' || value == '\t' || value == '\n' || value == '\r' || value == '\f' } // rewriteLinkHref rewrites href attributes in link tags From c28fb30fb1b7ef7550a6688d5ff74a1ce40a0dd0 Mon Sep 17 00:00:00 2001 From: marcomarcogd <35049765+marcomarcogd@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:33:15 +0800 Subject: [PATCH 2/2] =?UTF-8?q?fix=EF=BC=9A=E8=A1=A5=E9=BD=90=E8=AE=BA?= =?UTF-8?q?=E5=9D=9B=E6=87=92=E5=8A=A0=E8=BD=BD=E5=9B=BE=E7=89=87=E5=92=8C?= =?UTF-8?q?=E5=AA=92=E4=BD=93=E9=98=B2=E7=9B=97=E9=93=BE=E9=87=8D=E8=AF=95?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- internal/cache/media_cache.go | 78 +++++-- internal/cache/media_cache_test.go | 174 ++++++++++++++ internal/handlers/core/full_text.go | 22 +- internal/handlers/core/full_text_test.go | 66 ++++++ .../handlers/media/media_handlers_test.go | 58 +++++ internal/handlers/media/media_network_test.go | 60 +++++ internal/handlers/media/media_proxy.go | 171 +++++++------- internal/utils/httputil/httputil.go | 47 ++++ internal/utils/httputil/httputil_test.go | 219 ++++++++++++++++++ internal/utils/textutil/article_html.go | 37 ++- internal/utils/textutil/article_html_test.go | 62 +++++ 11 files changed, 873 insertions(+), 121 deletions(-) diff --git a/internal/cache/media_cache.go b/internal/cache/media_cache.go index 584dbea7e..5ba32e023 100644 --- a/internal/cache/media_cache.go +++ b/internal/cache/media_cache.go @@ -5,6 +5,7 @@ import ( "context" "crypto/sha256" "encoding/hex" + "errors" "fmt" "io" "net/http" @@ -14,6 +15,8 @@ import ( "sort" "strings" "time" + + "MrRSS/internal/utils/httputil" ) // MediaCache handles caching of images and videos to work around anti-hotlinking @@ -21,6 +24,16 @@ type MediaCache struct { cacheDir string } +// MediaDownloadError distinguishes a failed upstream fetch from a cache storage +// error, so callers do not repeat an already exhausted Referer fallback. +type MediaDownloadError struct { + Err error + RefererFallbackAttempted bool +} + +func (e *MediaDownloadError) Error() string { return e.Err.Error() } +func (e *MediaDownloadError) Unwrap() error { return e.Err } + // NewMediaCache creates a new media cache instance func NewMediaCache(cacheDir string) (*MediaCache, error) { // Create cache directory if it doesn't exist @@ -90,6 +103,7 @@ func (mc *MediaCache) Get(ctx context.Context, client *http.Client, url, referer if err != nil { return nil, "", fmt.Errorf("failed to download media: %w", err) } + cachedPath = mc.GetCachedPath(url) // Determine better file extension from Content-Type if available if contentType != "" { @@ -100,8 +114,24 @@ func (mc *MediaCache) Get(ctx context.Context, client *http.Client, url, referer } } - // Save to cache - if err := os.WriteFile(cachedPath, data, 0644); err != nil { + // Publish only a complete file, just as DownloadToFile does. + tmpFile, err := os.CreateTemp(mc.cacheDir, "download-*") + if err != nil { + return nil, "", fmt.Errorf("failed to create temporary cache file: %w", err) + } + defer os.Remove(tmpFile.Name()) + _, writeErr := tmpFile.Write(data) + closeErr := tmpFile.Close() + if writeErr != nil { + return nil, "", fmt.Errorf("failed to cache media: %w", writeErr) + } + if closeErr != nil { + return nil, "", fmt.Errorf("failed to finish cache file: %w", closeErr) + } + if err := ctx.Err(); err != nil { + return nil, "", err + } + if err := os.Rename(tmpFile.Name(), cachedPath); err != nil { return nil, "", fmt.Errorf("failed to cache media: %w", err) } @@ -140,14 +170,14 @@ func (mc *MediaCache) DownloadToFile(ctx context.Context, client *http.Client, u return "", "", err } - resp, err := client.Do(req) + resp, retried, err := httputil.DoWithRefererFallback(client, req) if err != nil { - return "", "", fmt.Errorf("failed to fetch media: %w", err) + return "", "", &MediaDownloadError{fmt.Errorf("failed to fetch media: %w", err), retried} } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - return "", "", fmt.Errorf("unexpected status code: %d", resp.StatusCode) + return "", "", &MediaDownloadError{fmt.Errorf("unexpected status code: %d", resp.StatusCode), retried} } tmpFile, err := os.CreateTemp(mc.cacheDir, "download-*") @@ -155,18 +185,25 @@ func (mc *MediaCache) DownloadToFile(ctx context.Context, client *http.Client, u return "", "", fmt.Errorf("failed to create temporary cache file: %w", err) } tmpPath := tmpFile.Name() + defer os.Remove(tmpPath) written, copyErr := io.Copy(tmpFile, resp.Body) closeErr := tmpFile.Close() - if copyErr != nil || closeErr != nil || written == 0 { - _ = os.Remove(tmpPath) - if copyErr != nil { - return "", "", fmt.Errorf("failed to stream media: %w", copyErr) + if copyErr != nil { + var pathErr *os.PathError + if errors.As(copyErr, &pathErr) { + return "", "", fmt.Errorf("failed to write cache file: %w", copyErr) } - if closeErr != nil { - return "", "", fmt.Errorf("failed to finish cache file: %w", closeErr) - } - return "", "", fmt.Errorf("empty media response") + return "", "", &MediaDownloadError{fmt.Errorf("failed to stream media: %w", copyErr), retried} + } + if closeErr != nil { + return "", "", fmt.Errorf("failed to finish cache file: %w", closeErr) + } + if err := ctx.Err(); err != nil { + return "", "", &MediaDownloadError{err, retried} + } + if written == 0 || (resp.ContentLength >= 0 && written != resp.ContentLength) { + return "", "", &MediaDownloadError{fmt.Errorf("incomplete media response: received %d bytes, expected %d", written, resp.ContentLength), retried} } contentType := resp.Header.Get("Content-Type") @@ -180,7 +217,6 @@ func (mc *MediaCache) DownloadToFile(ctx context.Context, client *http.Client, u } if err := os.Rename(tmpPath, finalPath); err != nil { - _ = os.Remove(tmpPath) return "", "", fmt.Errorf("failed to store media in cache: %w", err) } @@ -229,19 +265,25 @@ func (mc *MediaCache) download(ctx context.Context, client *http.Client, url, re return nil, "", err } - resp, err := client.Do(req) + resp, retried, err := httputil.DoWithRefererFallback(client, req) if err != nil { - return nil, "", fmt.Errorf("failed to fetch media: %w", err) + return nil, "", &MediaDownloadError{fmt.Errorf("failed to fetch media: %w", err), retried} } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - return nil, "", fmt.Errorf("unexpected status code: %d", resp.StatusCode) + return nil, "", &MediaDownloadError{fmt.Errorf("unexpected status code: %d", resp.StatusCode), retried} } data, err := io.ReadAll(resp.Body) if err != nil { - return nil, "", fmt.Errorf("failed to read response body: %w", err) + return nil, "", &MediaDownloadError{fmt.Errorf("failed to read response body: %w", err), retried} + } + if err := ctx.Err(); err != nil { + return nil, "", &MediaDownloadError{err, retried} + } + if len(data) == 0 || (resp.ContentLength >= 0 && int64(len(data)) != resp.ContentLength) { + return nil, "", &MediaDownloadError{fmt.Errorf("incomplete media response: received %d bytes, expected %d", len(data), resp.ContentLength), retried} } contentType := resp.Header.Get("Content-Type") diff --git a/internal/cache/media_cache_test.go b/internal/cache/media_cache_test.go index ea16ca7a0..d11dbd74d 100644 --- a/internal/cache/media_cache_test.go +++ b/internal/cache/media_cache_test.go @@ -3,15 +3,189 @@ package cache import ( "context" "errors" + "fmt" "io" "net/http" "net/http/httptest" "os" "path/filepath" + "strings" + "sync/atomic" "testing" "time" + + "MrRSS/internal/utils/httputil" ) +func TestMediaCacheRefererFallback(t *testing.T) { + for _, inMemory := range []bool{false, true} { + for _, finalStatus := range []int{http.StatusOK, http.StatusForbidden, http.StatusNotFound} { + name := fmt.Sprintf("memory=%v/status=%d", inMemory, finalStatus) + t.Run(name, func(t *testing.T) { + dir := t.TempDir() + mc, err := NewMediaCache(dir) + if err != nil { + t.Fatal(err) + } + var calls atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls.Add(1) + if r.Header.Get("Referer") != "" && finalStatus != http.StatusNotFound { + w.WriteHeader(http.StatusForbidden) + return + } + w.Header().Set("Content-Type", "image/png") + w.WriteHeader(finalStatus) + _, _ = io.WriteString(w, "image") + })) + defer server.Close() + mediaURL := server.URL + "/image.png" + if inMemory { + _, _, err = mc.Get(context.Background(), server.Client(), mediaURL, "https://article.example") + } else { + _, _, err = mc.DownloadToFile(context.Background(), server.Client(), mediaURL, "https://article.example") + } + wantCalls := int32(2) + if finalStatus == http.StatusNotFound { + wantCalls = 1 + } + if calls.Load() != wantCalls { + t.Fatalf("requests=%d want=%d", calls.Load(), wantCalls) + } + if finalStatus == http.StatusOK { + if err != nil { + t.Fatal(err) + } + data, contentType, err := mc.Get(context.Background(), server.Client(), mediaURL, "https://article.example") + if err != nil || string(data) != "image" || contentType != "image/png" || calls.Load() != wantCalls { + t.Fatalf("cache hit: data=%q type=%q err=%v requests=%d", data, contentType, err, calls.Load()) + } + } else { + var downloadErr *MediaDownloadError + if !errors.As(err, &downloadErr) || downloadErr.RefererFallbackAttempted != (wantCalls == 2) { + t.Fatalf("download error lost retry state: %v", err) + } + entries, err := os.ReadDir(dir) + if err != nil || len(entries) != 0 { + t.Fatalf("failed download left files: %v, %v", entries, err) + } + } + }) + } + } +} + +type failingMediaBody struct { + data string + err error + cancel context.CancelFunc + closed bool +} + +func (b *failingMediaBody) Read(p []byte) (int, error) { + if b.data != "" { + n := copy(p, b.data) + b.data = b.data[n:] + return n, nil + } + if b.cancel != nil { + b.cancel() + } + return 0, b.err +} + +func (b *failingMediaBody) Close() error { + b.closed = true + return nil +} + +func TestMediaCacheIncompleteOrCancelledBodyNeverCached(t *testing.T) { + for _, inMemory := range []bool{false, true} { + for _, mode := range []string{"short", "read failure", "cancelled", "empty"} { + t.Run(fmt.Sprintf("memory=%v/%s", inMemory, mode), func(t *testing.T) { + dir := t.TempDir() + mc, err := NewMediaCache(dir) + if err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + body := &failingMediaBody{data: "partial", err: io.EOF} + length := int64(10) + if mode == "read failure" { + body.err = io.ErrUnexpectedEOF + } else if mode == "cancelled" { + body.cancel = cancel + length = -1 + } else if mode == "empty" { + body.data = "" + length = 0 + } + calls := 0 + client := &http.Client{Transport: httputil.RoundTripFunc(func(req *http.Request) (*http.Response, error) { + calls++ + if calls == 1 { + return &http.Response{StatusCode: 403, Header: make(http.Header), Body: io.NopCloser(strings.NewReader("forbidden")), Request: req}, nil + } + return &http.Response{StatusCode: 200, Header: http.Header{"Content-Type": {"image/png"}}, Body: body, ContentLength: length, Request: req}, nil + })} + if inMemory { + _, _, err = mc.Get(ctx, client, "https://media.example/image.png", "https://article.example") + } else { + _, _, err = mc.DownloadToFile(ctx, client, "https://media.example/image.png", "https://article.example") + } + var downloadErr *MediaDownloadError + if !errors.As(err, &downloadErr) || !downloadErr.RefererFallbackAttempted || calls != 2 { + t.Fatalf("err=%v calls=%d", err, calls) + } + if mode == "cancelled" && !errors.Is(err, context.Canceled) { + t.Fatalf("cancellation lost: %v", err) + } + if !body.closed { + t.Fatal("failed response body was not closed") + } + entries, err := os.ReadDir(dir) + if err != nil || len(entries) != 0 { + t.Fatalf("incomplete download left files: %v, %v", entries, err) + } + }) + } + } +} + +func TestMediaCacheStorageErrorAllowsDirectFallback(t *testing.T) { + for _, inMemory := range []bool{false, true} { + t.Run(fmt.Sprintf("memory=%v", inMemory), func(t *testing.T) { + // A file in place of the cache directory makes cache storage fail + // deterministically, including when tests run as a privileged user. + cachePath := filepath.Join(t.TempDir(), "not-a-directory") + if err := os.WriteFile(cachePath, []byte("occupied"), 0600); err != nil { + t.Fatal(err) + } + mc := &MediaCache{cacheDir: cachePath} + calls := 0 + client := &http.Client{Transport: httputil.RoundTripFunc(func(req *http.Request) (*http.Response, error) { + calls++ + code := 200 + if calls == 1 { + code = 403 + } + return &http.Response{StatusCode: code, Header: http.Header{"Content-Type": {"image/png"}}, Body: io.NopCloser(strings.NewReader("image")), ContentLength: 5, Request: req}, nil + })} + var err error + if inMemory { + _, _, err = mc.Get(context.Background(), client, "https://media.example/image.png", "https://article.example") + } else { + _, _, err = mc.DownloadToFile(context.Background(), client, "https://media.example/image.png", "https://article.example") + } + var downloadErr *MediaDownloadError + if err == nil || errors.As(err, &downloadErr) || calls != 2 { + t.Fatalf("storage error incorrectly prevents direct fallback: err=%v calls=%d", err, calls) + } + }) + } +} + func TestMediaCacheCancelledDownloadDoesNotWriteFile(t *testing.T) { mc, err := NewMediaCache(t.TempDir()) if err != nil { diff --git a/internal/handlers/core/full_text.go b/internal/handlers/core/full_text.go index 17e5efb88..2395aa768 100644 --- a/internal/handlers/core/full_text.go +++ b/internal/handlers/core/full_text.go @@ -131,9 +131,9 @@ func (h *Handler) FetchFullArticleContentContext(ctx context.Context, articleURL // Explicit semantic article containers are a useful fallback for short pages. content, _ = doc.Find("article,main,[role=main]").First().Html() } - if !strings.Contains(strings.ToLower(content), "

` + content } } @@ -146,14 +146,13 @@ func (h *Handler) FetchFullArticleContentContext(ctx context.Context, articleURL func normalizeArticleImages(doc *goquery.Document, base *url.URL) { doc.Find("img").Each(func(_ int, img *goquery.Selection) { - source := "" - for _, attr := range []string{"data-src", "data-original", "data-lazy-src", "data-actualsrc", "data-original-src", "src"} { - value := strings.TrimSpace(img.AttrOr(attr, "")) - if value != "" && !strings.HasPrefix(value, "data:") { - source = value - break + attributes := make(map[string]string, len(img.Get(0).Attr)) + for _, attr := range img.Get(0).Attr { + if attr.Namespace == "" { + attributes[strings.ToLower(attr.Key)] = attr.Val } } + source := textutil.ResolveArticleImageSource(attributes, base) if source == "" { srcset := img.AttrOr("data-srcset", img.AttrOr("srcset", "")) candidates := strings.Split(srcset, ",") @@ -164,9 +163,14 @@ func normalizeArticleImages(doc *goquery.Document, base *url.URL) { } } } - if resolved, err := base.Parse(source); source != "" && err == nil && (resolved.Scheme == "http" || resolved.Scheme == "https") { + if resolved, err := base.Parse(source); source != "" && err == nil && resolved.Hostname() != "" && (resolved.Scheme == "http" || resolved.Scheme == "https") { img.SetAttr("src", resolved.String()) img.RemoveAttr("srcset").RemoveAttr("sizes").RemoveAttr("loading") + // Readability must not replace the selected source with another + // lazy attribute (for example a Discuz thumbnail on a lazy image). + for _, attr := range []string{"data-src", "data-original", "data-lazy-src", "data-actualsrc", "data-original-src", "data-srcset", "zoomfile", "file"} { + img.RemoveAttr(attr) + } } }) } diff --git a/internal/handlers/core/full_text_test.go b/internal/handlers/core/full_text_test.go index b57be4f3b..f3240bc53 100644 --- a/internal/handlers/core/full_text_test.go +++ b/internal/handlers/core/full_text_test.go @@ -7,8 +7,11 @@ import ( "fmt" "net/http" "net/http/httptest" + "net/url" "strings" "testing" + + "github.com/PuerkitoBio/goquery" ) func fullTextHandler(t *testing.T) *Handler { @@ -94,6 +97,10 @@ func TestFullTextReadabilityRestoresLeadImageWithoutDuplicates(t *testing.T) { fmt.Fprintf(w, `

Article

%s

`, body) return } + if r.URL.Path == "/no-lead" { + fmt.Fprintf(w, `

Article

%s

`, body) + return + } fmt.Fprintf(w, `

Article

%s

`, body) })) defer server.Close() @@ -113,6 +120,65 @@ func TestFullTextReadabilityRestoresLeadImageWithoutDuplicates(t *testing.T) { if !strings.Contains(content, server.URL+"/body.jpg") || strings.Contains(content, server.URL+"/og.jpg") { t.Fatalf("existing article image was duplicated or replaced: %s", content) } + content, err = h.FetchFullArticleContentContext(context.Background(), server.URL+"/no-lead", nil) + if err != nil || strings.Contains(content, "Forum article

Forum article

%s

`, strings.Repeat("An article with useful details and punctuation, describing a local event. ", 30)) + })) + defer server.Close() + content, err := h.FetchFullArticleContentContext(context.Background(), server.URL+"/article", nil) + if err != nil { + t.Fatal(err) + } + doc, err := goquery.NewDocumentFromReader(strings.NewReader(content)) + if err != nil { + t.Fatal(err) + } + want := []string{ + "https://att.huarenjie.com/attachment/forum/202609/29/004010ve8fzabf4ee88kua.jpg", + server.URL + "/full.jpg?size=large&type=image", + } + if doc.Find("img").Length() != len(want) { + t.Fatalf("expected both Discuz attachments: %s", content) + } + doc.Find("img").Each(func(i int, image *goquery.Selection) { + if image.AttrOr("src", "") != want[i] || image.AttrOr("referrerpolicy", "") != "no-referrer" { + t.Errorf("unexpected image %d: %s", i, content) + } + }) + for _, unwanted := range []string{"none.gif", "thumb.jpg", "zoomfile", "javascript:", "onclick"} { + if strings.Contains(content, unwanted) { + t.Errorf("unexpected %q in full text: %s", unwanted, content) + } + } +} + +func TestNormalizeArticleImagesPreservesOrdinaryAndSrcsetSources(t *testing.T) { + base, _ := url.Parse("https://example.org/news/article") + doc, err := goquery.NewDocumentFromReader(strings.NewReader(``)) + if err != nil { + t.Fatal(err) + } + normalizeArticleImages(doc, base) + want := []string{ + "https://example.org/ordinary.jpg", + "data:image/png;base64,aGVsbG8=", + "https://example.org/news/large.jpg", + "https://example.org/news/large.jpg", + "https://example.org/data.jpg", + } + doc.Find("img").Each(func(i int, image *goquery.Selection) { + if got := image.AttrOr("src", ""); got != want[i] { + t.Errorf("image %d: got %q, want %q", i, got, want[i]) + } + }) } func TestFullTextNoSelectorMatchIsAnError(t *testing.T) { diff --git a/internal/handlers/media/media_handlers_test.go b/internal/handlers/media/media_handlers_test.go index 672317217..a15a642ea 100644 --- a/internal/handlers/media/media_handlers_test.go +++ b/internal/handlers/media/media_handlers_test.go @@ -2,8 +2,10 @@ package media import ( "encoding/base64" + "github.com/PuerkitoBio/goquery" "net/http" "net/http/httptest" + "net/url" "regexp" "strings" "testing" @@ -242,3 +244,59 @@ func findInString(s, substr string) bool { } return false } + +func TestRewriteHTMLContentDiscuzAttachments(t *testing.T) { + base := "https://forum.example.org/thread-1.html" + input := `` + result := rewriteHTMLContent([]byte(input), base) + doc, err := goquery.NewDocumentFromReader(strings.NewReader(string(result))) + if err != nil { + t.Fatal(err) + } + expected := []string{"https://cdn.example.org/large.jpg?a=1&b=2", "https://forum.example.org/real.jpg", "https://forum.example.org/preferred.jpg", "https://forum.example.org/valid.jpg"} + if doc.Find("img").Length() != len(expected) { + t.Fatalf("unexpected image count: %s", result) + } + doc.Find("img").Each(func(i int, img *goquery.Selection) { + for _, attr := range []string{"src", "zoomfile", "file"} { + raw, exists := img.Attr(attr) + if !exists { + continue + } + parsed, err := url.Parse(raw) + if err != nil { + t.Fatal(err) + } + if parsed.Path != "/api/webpage/resource" { + t.Errorf("%s is not proxied: %s", attr, raw) + continue + } + decoded, err := base64.StdEncoding.DecodeString(parsed.Query().Get("url_b64")) + if err != nil { + t.Fatal(err) + } + if attr == "src" && string(decoded) != expected[i] { + t.Errorf("image %d src=%s, want %s", i, decoded, expected[i]) + } + if strings.Contains(string(decoded), "none.gif") || strings.Contains(string(decoded), "javascript:") || strings.Contains(string(decoded), "missing.jpg") { + t.Errorf("invalid attachment source: %s", decoded) + } + } + }) +} + +func TestConvertLazyImagesKeepsExistingProxies(t *testing.T) { + proxy := "/api/webpage/resource?url_b64=" + base64.StdEncoding.EncodeToString([]byte("https://cdn.example.org/photo.jpg")) + "&referer_b64=" + base64.StdEncoding.EncodeToString([]byte("https://forum.example.org/thread")) + for _, input := range []string{``, ``} { + output := convertLazyImages(input, "https://forum.example.org/thread") + output = convertLazyImages(output, "https://forum.example.org/thread") + doc, err := goquery.NewDocumentFromReader(strings.NewReader(output)) + if err != nil { + t.Fatal(err) + } + img := doc.Find("img").First() + if img.AttrOr("src", "") != proxy || img.AttrOr("zoomfile", "") != proxy { + t.Errorf("local proxy changed on repeat: %s", output) + } + } +} diff --git a/internal/handlers/media/media_network_test.go b/internal/handlers/media/media_network_test.go index d0cc80e3e..bc1c9d539 100644 --- a/internal/handlers/media/media_network_test.go +++ b/internal/handlers/media/media_network_test.go @@ -3,6 +3,7 @@ package media import ( "context" "encoding/base64" + "fmt" "net" "net/http" "net/http/httptest" @@ -118,3 +119,62 @@ func TestMediaProxyDisabledApplicationProxy(t *testing.T) { t.Fatal("cancelled download succeeded") } } + +func TestMediaProxyRefererFallbackAndExhaustion(t *testing.T) { + for _, cacheEnabled := range []bool{false, true} { + for _, outcome := range []string{"first-success", "retry-success", "forbidden"} { + t.Run(fmt.Sprintf("cache=%t/%s", cacheEnabled, outcome), func(t *testing.T) { + tmp := t.TempDir() + t.Setenv("APPDATA", tmp) + t.Setenv("HOME", tmp) + t.Setenv("XDG_DATA_HOME", tmp) + h := setupHandler(t) + defer h.DB.Close() + if err := h.DB.SetSetting("media_cache_enabled", fmt.Sprint(cacheEnabled)); err != nil { + t.Fatal(err) + } + if err := h.DB.SetSetting("media_proxy_fallback", "true"); err != nil { + t.Fatal(err) + } + var requests atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + n := requests.Add(1) + if n == 1 && r.Header.Get("Referer") == "" { + t.Error("first request lost original referer") + } + if outcome == "forbidden" || (outcome == "retry-success" && r.Header.Get("Referer") != "") { + w.WriteHeader(http.StatusForbidden) + return + } + w.Header().Set("Content-Type", "image/png") + _, _ = w.Write([]byte("image data")) + })) + defer server.Close() + address := "/api/media/proxy?url=" + url.QueryEscape(server.URL+"/photo.png") + "&referer=" + url.QueryEscape(server.URL+"/article") + rec := httptest.NewRecorder() + HandleMediaProxy(h, rec, httptest.NewRequest(http.MethodGet, address, nil)) + wantStatus := http.StatusOK + wantRequests := int32(1) + if outcome != "first-success" { + wantRequests = 2 + } + if outcome == "forbidden" { + wantStatus = http.StatusInternalServerError + } + if rec.Code != wantStatus || requests.Load() != wantRequests { + t.Fatalf("status=%d requests=%d, want status=%d requests=%d; %s", rec.Code, requests.Load(), wantStatus, wantRequests, rec.Body.String()) + } + if wantStatus == http.StatusOK && (rec.Body.String() != "image data" || rec.Header().Get("Content-Type") != "image/png") { + t.Fatalf("invalid image response: %s", rec.Body.String()) + } + if cacheEnabled && wantStatus == http.StatusOK { + cached := httptest.NewRecorder() + HandleMediaProxy(h, cached, httptest.NewRequest(http.MethodGet, address, nil)) + if cached.Code != http.StatusOK || requests.Load() != wantRequests { + t.Fatal("second load did not use successful cache") + } + } + }) + } + } +} diff --git a/internal/handlers/media/media_proxy.go b/internal/handlers/media/media_proxy.go index b001e666d..284b30e69 100644 --- a/internal/handlers/media/media_proxy.go +++ b/internal/handlers/media/media_proxy.go @@ -23,6 +23,9 @@ import ( "MrRSS/internal/handlers/response" "MrRSS/internal/utils/fileutil" "MrRSS/internal/utils/httputil" + "MrRSS/internal/utils/textutil" + + htmlparser "golang.org/x/net/html" ) // validateMediaURL validates that the URL is HTTP/HTTPS and properly formatted @@ -162,6 +165,9 @@ func getSmartReferer(imageURL, originalReferer string) string { // @Failure 500 {object} map[string]string "Internal server error" // @Router /media/proxy [get] func HandleMediaProxy(h *core.Handler, w http.ResponseWriter, r *http.Request) { + ctx, cancel := context.WithTimeout(r.Context(), 30*time.Second) + defer cancel() + r = r.WithContext(ctx) if r.Method != http.MethodGet { response.Error(w, nil, http.StatusMethodNotAllowed) return @@ -266,6 +272,11 @@ func HandleMediaProxy(h *core.Handler, w http.ResponseWriter, r *http.Request) { _ = file.Close() } } + var downloadErr *cache.MediaDownloadError + if errors.As(err, &downloadErr) && downloadErr.RefererFallbackAttempted { + response.Error(w, fmt.Errorf("failed to fetch media"), http.StatusInternalServerError) + return + } log.Printf("Cache failed for %s: %v, trying fallback", mediaURL, err) } } @@ -819,7 +830,7 @@ func rewriteHTMLContent(bodyBytes []byte, baseURL string) []byte { // First, convert lazy-loaded images to normal images // This ensures images load immediately without waiting for lazy loading scripts - content = convertLazyImages(content) + content = convertLazyImages(content, baseURL) // Then rewrite img src attributes (now including the converted lazy images) content = rewriteAttribute(content, "img", "src", baseURL) @@ -865,102 +876,84 @@ func rewriteHTMLContent(bodyBytes []byte, baseURL string) []byte { return []byte(content) } -// convertLazyImages converts lazy-loaded images to normal images -// For images with data-original or data-src attributes, move those URLs to src -// This prevents lazy loading and ensures immediate display -func convertLazyImages(content string) string { - // Match img tags with lazy loading attributes - // We need to match any img tag that contains data-original or data-src - // Use a two-step approach: find all img tags, then check if they have lazy attributes - re := regexp.MustCompile(`]*>`) - - return re.ReplaceAllStringFunc(content, func(match string) string { - // Check if this img tag has data-original or data-src attribute - // Try double quotes first: data-original="..." - doubleQuoteRe := regexp.MustCompile(`\s(data-original|data-src)\s*=\s*"([^"]*)"`) - doubleQuoteMatch := doubleQuoteRe.FindStringSubmatch(match) - - var lazySrc, lazyQuote string - - if len(doubleQuoteMatch) >= 3 { - // Found double-quoted attribute - lazySrc = doubleQuoteMatch[2] - lazyQuote = `"` - } else { - // Try single quotes: data-original='...' - singleQuoteRe := regexp.MustCompile(`\s(data-original|data-src)\s*=\s*'([^']*)'`) - singleQuoteMatch := singleQuoteRe.FindStringSubmatch(match) - if len(singleQuoteMatch) >= 3 { - lazySrc = singleQuoteMatch[2] - lazyQuote = `'` - } else { - // Try unquoted: data-original=... - unquotedRe := regexp.MustCompile(`\s(data-original|data-src)\s*=\s*([^\s>]+)`) - unquotedMatch := unquotedRe.FindStringSubmatch(match) - if len(unquotedMatch) >= 3 { - lazySrc = unquotedMatch[2] - lazyQuote = "" - } else { - // No lazy attribute found - return match - } - } +// convertLazyImages resolves lazy image sources before rewriting resource URLs. +// Keep Discuz attachment attributes proxied too, because its scripts may reuse them. +func convertLazyImages(content, baseURL string) string { + base, err := url.Parse(baseURL) + if err != nil { + return content + } + images := regexp.MustCompile(`(?is)]*>`) + return images.ReplaceAllStringFunc(content, func(match string) string { + tokenizer := htmlparser.NewTokenizer(strings.NewReader(match)) + kind := tokenizer.Next() + if kind != htmlparser.StartTagToken && kind != htmlparser.SelfClosingTagToken { + return match } - - // Build new img tag - var newTag strings.Builder - newTag.WriteString("") - - return newTag.String() + token.Attr = append(attrs, htmlparser.Attribute{Key: "src", Val: source}) + return token.String() }) } +func isLocalImageProxy(value string) bool { + return strings.HasPrefix(value, "/api/webpage/resource?") || strings.HasPrefix(value, "/api/media/proxy?") +} + // htmlAttribute represents a parsed HTML attribute type htmlAttribute struct { Name string @@ -1144,7 +1137,7 @@ func proxyWebpageResourceURL(value, baseURL string) (string, bool) { resolvedURL := resolveURL(value, baseURL) parsedURL, err := url.Parse(resolvedURL) - if err != nil || (parsedURL.Scheme != "http" && parsedURL.Scheme != "https") { + if err != nil || parsedURL.Hostname() == "" || (parsedURL.Scheme != "http" && parsedURL.Scheme != "https") { return value, false } return fmt.Sprintf("/api/webpage/resource?url_b64=%s&referer_b64=%s", @@ -1814,7 +1807,7 @@ func proxyMediaDirectly(ctx context.Context, client *http.Client, mediaURL, refe req.Header.Set("Accept", "image/webp,image/apng,image/*,*/*;q=0.8") req.Header.Set("Accept-Language", "en-US,en;q=0.9") - resp, err := client.Do(req) + resp, _, err := httputil.DoWithRefererFallback(client, req) if err != nil { return fmt.Errorf("failed to fetch media: %w", err) } diff --git a/internal/utils/httputil/httputil.go b/internal/utils/httputil/httputil.go index 0303bb476..681d2dc99 100644 --- a/internal/utils/httputil/httputil.go +++ b/internal/utils/httputil/httputil.go @@ -113,6 +113,53 @@ func insecureSkipTLSVerifyEnabled() bool { } } +// DoWithRefererFallback retries a rejected media GET once without Referer. It +// uses the request's context for both attempts and leaves the shared client and +// original request headers unchanged. The caller owns the returned response. +func DoWithRefererFallback(client *http.Client, req *http.Request) (*http.Response, bool, error) { + if err := req.Context().Err(); err != nil { + return nil, false, err + } + canRetry := req.Method == http.MethodGet && req.Header.Get("Referer") != "" + resp, err := client.Do(req) + if err != nil || !canRetry || resp.StatusCode != http.StatusForbidden { + return resp, false, err + } + // A media request has no body. Do not replay an unrelated GET with a body + // unless its caller provided a way to recreate it. + if req.Body != nil && req.Body != http.NoBody && req.GetBody == nil { + return resp, false, nil + } + _ = resp.Body.Close() + if err := req.Context().Err(); err != nil { + return nil, false, err + } + + retryReq := req.Clone(req.Context()) + retryReq.Header.Del("Referer") + if req.GetBody != nil { + retryReq.Body, err = req.GetBody() + if err != nil { + return nil, false, err + } + } + retryClient := *client + retryClient.CheckRedirect = func(next *http.Request, via []*http.Request) error { + var err error + if client.CheckRedirect != nil { + err = client.CheckRedirect(next, via) + } else if len(via) >= 10 { + err = fmt.Errorf("stopped after 10 redirects") + } + // net/http synthesizes Referer on redirects, and a caller's redirect + // callback can set it too. Remove it after applying the original policy. + next.Header.Del("Referer") + return err + } + resp, err = retryClient.Do(retryReq) + return resp, true, err +} + // CreateHTTPClientWithUserAgent creates an HTTP client with custom User-Agent. func CreateHTTPClientWithUserAgent(proxyURL string, timeout time.Duration, userAgent string) (*http.Client, error) { baseClient, err := CreateHTTPClient(proxyURL, timeout) diff --git a/internal/utils/httputil/httputil_test.go b/internal/utils/httputil/httputil_test.go index 000399f29..df3ab8512 100644 --- a/internal/utils/httputil/httputil_test.go +++ b/internal/utils/httputil/httputil_test.go @@ -1,15 +1,234 @@ package httputil import ( + "context" "crypto/tls" + "errors" "io" "net/http" "net/http/httptest" "net/url" + "strings" + "sync/atomic" "testing" "time" ) +type refererFallbackBody struct { + io.Reader + closed bool +} + +func (b *refererFallbackBody) Close() error { + b.closed = true + return nil +} + +func TestDoWithRefererFallbackConditions(t *testing.T) { + for _, tc := range []struct { + name string + method string + referer string + status int + finalCode int + wantRetry bool + }{ + {"success", http.MethodGet, "https://article.example", 200, 200, false}, + {"forbidden", http.MethodGet, "https://article.example", 403, 200, true}, + {"still forbidden", http.MethodGet, "https://article.example", 403, 403, true}, + {"no referer", http.MethodGet, "", 403, 403, false}, + {"not GET", http.MethodHead, "https://article.example", 403, 403, false}, + {"not found", http.MethodGet, "https://article.example", 404, 404, false}, + {"rate limited", http.MethodGet, "https://article.example", 429, 429, false}, + {"server error", http.MethodGet, "https://article.example", 500, 500, false}, + } { + t.Run(tc.name, func(t *testing.T) { + firstBody := &refererFallbackBody{Reader: strings.NewReader("first")} + calls := 0 + client := &http.Client{Transport: RoundTripFunc(func(req *http.Request) (*http.Response, error) { + calls++ + if calls == 1 { + return &http.Response{StatusCode: tc.status, Body: firstBody, Header: make(http.Header), Request: req}, nil + } + if !firstBody.closed { + t.Error("first response body was not closed before retry") + } + if req.Header.Get("Referer") != "" || req.Header.Get("User-Agent") != "media-test" { + t.Errorf("unexpected retry headers: %v", req.Header) + } + return &http.Response{StatusCode: tc.finalCode, Body: io.NopCloser(strings.NewReader("image")), Header: make(http.Header), Request: req}, nil + })} + req, err := http.NewRequestWithContext(context.Background(), tc.method, "https://media.example/image", nil) + if err != nil { + t.Fatal(err) + } + req.Header.Set("Referer", tc.referer) + req.Header.Set("User-Agent", "media-test") + resp, retried, err := DoWithRefererFallback(client, req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + wantCalls := 1 + if tc.wantRetry { + wantCalls++ + } + if retried != tc.wantRetry || calls != wantCalls || resp.StatusCode != tc.finalCode { + t.Fatalf("retry=%v calls=%d status=%d", retried, calls, resp.StatusCode) + } + if req.Header.Get("Referer") != tc.referer || client.CheckRedirect != nil { + t.Fatal("helper changed original request or shared client") + } + if !tc.wantRetry && firstBody.closed { + t.Fatal("helper closed response that belongs to caller") + } + }) + } +} + +func TestDoWithRefererFallbackErrorsAndCancellation(t *testing.T) { + for _, mode := range []string{"network", "cancelled before request", "cancelled after forbidden", "cancelled during retry"} { + t.Run(mode, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + calls := 0 + firstBody := &refererFallbackBody{Reader: strings.NewReader("forbidden")} + networkErr := errors.New("network unavailable") + client := &http.Client{Transport: RoundTripFunc(func(req *http.Request) (*http.Response, error) { + calls++ + if mode == "network" { + return nil, networkErr + } + if mode == "cancelled after forbidden" { + cancel() + } + if calls == 2 { + cancel() + return nil, req.Context().Err() + } + return &http.Response{StatusCode: 403, Body: firstBody, Header: make(http.Header), Request: req}, nil + })} + if mode == "cancelled before request" { + cancel() + } + req, err := http.NewRequestWithContext(ctx, http.MethodGet, "https://media.example/image", nil) + if err != nil { + t.Fatal(err) + } + req.Header.Set("Referer", "https://article.example") + resp, retried, err := DoWithRefererFallback(client, req) + if resp != nil { + resp.Body.Close() + } + wantErr := error(context.Canceled) + wantCalls := 1 + if mode == "network" { + wantErr = networkErr + } else if mode == "cancelled before request" { + wantCalls = 0 + } else if mode == "cancelled during retry" { + wantCalls = 2 + } + if !errors.Is(err, wantErr) || calls != wantCalls || retried != (wantCalls == 2) { + t.Fatalf("err=%v calls=%d retried=%v", err, calls, retried) + } + if strings.HasPrefix(mode, "cancelled after") || strings.HasPrefix(mode, "cancelled during") { + if !firstBody.closed { + t.Fatal("forbidden body was not closed") + } + } + }) + } +} + +func TestDoWithRefererFallbackClearsRedirectReferer(t *testing.T) { + var calls atomic.Int32 + var redirects atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls.Add(1) + if r.URL.Path == "/image" && r.Header.Get("Referer") != "" { + w.WriteHeader(http.StatusForbidden) + return + } + if r.Header.Get("Referer") != "" { + t.Errorf("retry or redirect retained Referer %q", r.Header.Get("Referer")) + } + if r.URL.Path == "/image" { + http.Redirect(w, r, "/final", http.StatusFound) + return + } + if r.Header.Get("X-Redirect-Policy") != "preserved" { + t.Error("original redirect callback was not applied") + } + _, _ = io.WriteString(w, "image") + })) + defer server.Close() + client := server.Client() + client.CheckRedirect = func(req *http.Request, via []*http.Request) error { + redirects.Add(1) + req.Header.Set("Referer", "https://must-be-removed.example") + req.Header.Set("X-Redirect-Policy", "preserved") + return nil + } + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, server.URL+"/image", nil) + if err != nil { + t.Fatal(err) + } + req.Header.Set("Referer", "https://article.example") + resp, retried, err := DoWithRefererFallback(client, req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + if !retried || resp.StatusCode != 200 || calls.Load() != 3 || redirects.Load() != 1 { + t.Fatalf("retry=%v status=%d requests=%d redirects=%d", retried, resp.StatusCode, calls.Load(), redirects.Load()) + } + probe, _ := http.NewRequest(http.MethodGet, server.URL, nil) + if err := client.CheckRedirect(probe, nil); err != nil || probe.Header.Get("Referer") == "" { + t.Fatal("shared client's redirect callback was changed") + } +} + +func TestDoWithRefererFallbackPreservesRedirectLimits(t *testing.T) { + for _, custom := range []bool{false, true} { + t.Run(map[bool]string{false: "default", true: "custom"}[custom], func(t *testing.T) { + var calls atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls.Add(1) + if r.Header.Get("Referer") != "" { + w.WriteHeader(http.StatusForbidden) + return + } + http.Redirect(w, r, "/loop", http.StatusFound) + })) + defer server.Close() + client := server.Client() + policyErr := errors.New("redirect rejected by policy") + if custom { + client.CheckRedirect = func(*http.Request, []*http.Request) error { return policyErr } + } + req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, server.URL+"/image", nil) + req.Header.Set("Referer", "https://article.example") + resp, retried, err := DoWithRefererFallback(client, req) + if resp != nil { + resp.Body.Close() + } + wantCalls := int32(11) + if custom { + wantCalls = 2 + if !errors.Is(err, policyErr) { + t.Fatalf("redirect policy error lost: %v", err) + } + } else if err == nil || !strings.Contains(err.Error(), "stopped after 10 redirects") { + t.Fatalf("default redirect limit lost: %v", err) + } + if !retried || calls.Load() != wantCalls { + t.Fatalf("retry=%v calls=%d want=%d", retried, calls.Load(), wantCalls) + } + }) + } +} + func TestBuildProxyURLPreservesCredentialsAndIPv6(t *testing.T) { for _, host := range []string{"127.0.0.1", "::1", "[::1]"} { raw := BuildProxyURL("http", host, "7890", "user@company", "p@ss:/?#%") diff --git a/internal/utils/textutil/article_html.go b/internal/utils/textutil/article_html.go index f60beaa21..89dbfc320 100644 --- a/internal/utils/textutil/article_html.go +++ b/internal/utils/textutil/article_html.go @@ -13,6 +13,30 @@ var markdownBlock = regexp.MustCompile("(?m)^ {0,3}(#{1,6} |```|~~~|[-*+] |[0-9] var rasterDataImage = regexp.MustCompile(`(?i)^data:image/(png|gif|jpeg|webp|avif);base64,[a-z0-9+/=\r\n]+$`) var htmlElement = regexp.MustCompile(`(?i)<[a-z][a-z0-9]*(?:\s|/?>)`) +// ResolveArticleImageSource selects the first valid HTTP(S) image source. +// Attribute names must be lowercase and values must already be HTML-decoded. +// Inline images are left to the caller, which can preserve the existing src. +func ResolveArticleImageSource(attributes map[string]string, base *url.URL) string { + for _, key := range []string{"data-src", "data-original", "data-lazy-src", "data-actualsrc", "data-original-src", "zoomfile", "file", "src"} { + value := strings.TrimSpace(attributes[key]) + if value == "" || strings.HasPrefix(value, "#") { + continue + } + source, err := url.Parse(value) + if err != nil { + continue + } + if base != nil { + source = base.ResolveReference(source) + } + if (source.Scheme != "http" && source.Scheme != "https") || source.Hostname() == "" { + continue + } + return source.String() + } + return "" +} + // PrepareArticleContent renders Markdown sources and normalizes safe reader HTML. // Existing HTML and pre/code examples remain HTML rather than being reinterpreted. func PrepareArticleContent(content, baseURL string) string { @@ -24,16 +48,19 @@ func PrepareArticleContent(content, baseURL string) string { return "" } doc.Find("script,style,link,meta,base,object,embed,form,input,button,textarea,select,svg,template").Remove() + base, _ := url.Parse(baseURL) // Promote lazy image URLs in RSS HTML too, before removing source attributes. doc.Find("img").Each(func(_ int, image *goquery.Selection) { - for _, key := range []string{"data-src", "data-original", "data-lazy-src", "data-actualsrc"} { - if value := strings.TrimSpace(image.AttrOr(key, "")); value != "" { - image.SetAttr("src", value) - break + attributes := make(map[string]string, len(image.Get(0).Attr)) + for _, attr := range image.Get(0).Attr { + if attr.Namespace == "" { + attributes[strings.ToLower(attr.Key)] = attr.Val } } + if source := ResolveArticleImageSource(attributes, base); source != "" { + image.SetAttr("src", source) + } }) - base, _ := url.Parse(baseURL) doc.Find("*").Each(func(_ int, sel *goquery.Selection) { node := sel.Get(0) if node.Type != html.ElementNode { diff --git a/internal/utils/textutil/article_html_test.go b/internal/utils/textutil/article_html_test.go index 2990170d0..dee314da4 100644 --- a/internal/utils/textutil/article_html_test.go +++ b/internal/utils/textutil/article_html_test.go @@ -1,8 +1,11 @@ package textutil import ( + "net/url" "strings" "testing" + + "github.com/PuerkitoBio/goquery" ) func TestArticleMarkdownAndCodeFormatting(t *testing.T) { @@ -74,3 +77,62 @@ func TestArticleImagesDefaultToNoReferrer(t *testing.T) { } } } + +func TestArticleImageSourcePriorityAndValidation(t *testing.T) { + base, _ := url.Parse("https://example.org/news/article") + for _, test := range []struct { + name string + attributes map[string]string + want string + }{ + {"existing lazy priority", map[string]string{"data-src": "/data.jpg", "data-original": "/original.jpg", "zoomfile": "/full.jpg", "src": "/none.gif"}, "https://example.org/data.jpg"}, + {"original lazy priority", map[string]string{"data-original": "/original.jpg", "data-lazy-src": "/lazy.jpg"}, "https://example.org/original.jpg"}, + {"remaining lazy priority", map[string]string{"data-lazy-src": "/lazy.jpg", "data-actualsrc": "/actual.jpg", "data-original-src": "/original-src.jpg"}, "https://example.org/lazy.jpg"}, + {"actual before original src", map[string]string{"data-actualsrc": "/actual.jpg", "data-original-src": "/original-src.jpg"}, "https://example.org/actual.jpg"}, + {"original src before discuz", map[string]string{"data-original-src": "/original-src.jpg", "zoomfile": "/full.jpg"}, "https://example.org/original-src.jpg"}, + {"discuz full before thumbnail", map[string]string{"zoomfile": "../full.jpg", "file": "/thumb.jpg", "src": "/none.gif"}, "https://example.org/full.jpg"}, + {"invalid candidates continue", map[string]string{"data-src": "javascript:alert(1)", "data-original": "http:///missing-host.jpg", "zoomfile": "data:image/png;base64,AAAA", "file": "//cdn.example/photo.jpg", "src": "/none.gif"}, "https://cdn.example/photo.jpg"}, + {"malformed candidates continue", map[string]string{"data-src": "%zz", "zoomfile": "#image", "file": "blob:https://example.org/id", "src": "/photo.jpg"}, "https://example.org/photo.jpg"}, + {"already decoded entity remains literal", map[string]string{"zoomfile": "/image?a=1&b=2"}, "https://example.org/image?a=1&b=2"}, + {"ordinary source", map[string]string{"src": "images/photo.jpg"}, "https://example.org/news/images/photo.jpg"}, + {"inline source preserved by caller", map[string]string{"src": "data:image/png;base64,aGVsbG8="}, ""}, + {"no source", map[string]string{}, ""}, + } { + t.Run(test.name, func(t *testing.T) { + if got := ResolveArticleImageSource(test.attributes, base); got != test.want { + t.Errorf("got %q, want %q", got, test.want) + } + }) + } + if got := ResolveArticleImageSource(map[string]string{"zoomfile": "/relative.jpg", "file": "https://cdn.example/full.jpg"}, nil); got != "https://cdn.example/full.jpg" { + t.Errorf("nil base must skip relative candidates: %q", got) + } +} + +func TestArticleHTMLPromotesDiscuzImagesBeforeSanitizing(t *testing.T) { + content := `

Article body.

` + got := PrepareArticleContent(content, "https://example.org/news/article") + doc, err := goquery.NewDocumentFromReader(strings.NewReader(got)) + if err != nil { + t.Fatal(err) + } + want := []string{ + "https://att.huarenjie.com/attachment/forum/202609/29/004010ve8fzabf4ee88kua.jpg", + "https://example.org/full.jpg?size=large&type=image", + "data:image/png;base64,aGVsbG8=", + "https://example.org/original.jpg", + } + if doc.Find("img").Length() != len(want) { + t.Fatalf("unexpected image count: %s", got) + } + doc.Find("img").Each(func(i int, image *goquery.Selection) { + if image.AttrOr("src", "") != want[i] || image.AttrOr("referrerpolicy", "") != "no-referrer" { + t.Errorf("unexpected image %d: %s", i, got) + } + }) + for _, unsafe := range []string{"onclick", "zoomfile", "file=", "javascript:", "none.gif", ".thumb.jpg", "data-original-src"} { + if strings.Contains(got, unsafe) { + t.Errorf("unexpected %q in sanitized article: %s", unsafe, got) + } + } +}