From 067aab4f9aa514174367c7822fadc3fc56431fb8 Mon Sep 17 00:00:00 2001 From: wraithfive Date: Sat, 1 Aug 2026 20:49:46 -0700 Subject: [PATCH 1/5] Fix: CDP scrapers return HTML-wrapped JSON instead of raw response for JSON content types When a useCDP scraper navigates directly to a URL that Chrome identifies as JSON content, Chrome's own JSON viewer wraps the raw JSON in an HTML pretty-printer document before it reaches the page DOM. urlFromCDP always extracted content via OuterHTML, so scrapeJson-type scrapers using useCDP would receive this HTML wrapper instead of the JSON itself and fail with "not valid json". Detect when the main document response's MIME type is JSON and, in that case, pull the raw body via Network.getResponseBody instead of reading the rendered DOM. Non-JSON responses are unaffected and continue to use OuterHTML as before. --- pkg/scraper/url.go | 48 ++++++++++++++++++++++++++++++++++++++++- pkg/scraper/url_test.go | 33 ++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 pkg/scraper/url_test.go diff --git a/pkg/scraper/url.go b/pkg/scraper/url.go index d95f4563f1..c201e84cd2 100644 --- a/pkg/scraper/url.go +++ b/pkg/scraper/url.go @@ -253,6 +253,25 @@ func urlFromCDP(ctx context.Context, urlCDP string, driverOptions scraperDriverO var res string headers := cdpHeaders(driverOptions) + // Track the main document response so that, if it turns out to be JSON, + // we can pull the raw response body via CDP's Network domain instead of + // reading the rendered DOM. Chrome's built-in JSON viewer wraps raw JSON + // responses in an HTML pretty-printer when navigated to directly, so + // OuterHTML on such a page returns HTML containing the JSON rather than + // the JSON itself. + var jsonRequestID network.RequestID + var isJSONDocument bool + chromedp.ListenTarget(ctx, func(ev interface{}) { + if ev, ok := ev.(*network.EventResponseReceived); ok { + if ev.Type == network.ResourceTypeDocument && !isJSONDocument { + if isJSONMimeType(ev.Response.MimeType) { + jsonRequestID = ev.RequestID + isJSONDocument = true + } + } + } + }) + if proxyUsesAuth(globalConfig.GetProxy()) { _, user, pass := splitProxyAuth(globalConfig.GetProxy()) @@ -294,7 +313,20 @@ func urlFromCDP(ctx context.Context, urlCDP string, driverOptions scraperDriverO chromedp.Navigate(urlCDP), chromedp.Sleep(sleepDuration), setCDPClicks(driverOptions), - chromedp.OuterHTML("html", &res, chromedp.ByQuery), + chromedp.ActionFunc(func(ctx context.Context) error { + if isJSONDocument { + body, err := network.GetResponseBody(jsonRequestID).Do(ctx) + if err != nil { + // fall back to OuterHTML if the response body is no + // longer available (e.g. evicted from Chrome's cache) + logger.Debugf("[scraper] could not get raw response body for JSON document, falling back to OuterHTML: %v", err) + return chromedp.OuterHTML("html", &res, chromedp.ByQuery).Do(ctx) + } + res = string(body) + return nil + } + return chromedp.OuterHTML("html", &res, chromedp.ByQuery).Do(ctx) + }), printCDPCookies(driverOptions, "Cookies set"), ) @@ -361,6 +393,20 @@ func getRemoteCDPWSAddress(ctx context.Context, url string) (string, error) { return remote, err } +// isJSONMimeType returns true if the given response MIME type indicates +// JSON content (e.g. "application/json", "application/ld+json", +// "text/json; charset=utf-8"), for deciding whether a CDP-fetched document +// should be read via its raw network response body rather than the +// rendered DOM. +func isJSONMimeType(mimeType string) bool { + mimeType, _, _ = strings.Cut(mimeType, ";") + mimeType = strings.TrimSpace(strings.ToLower(mimeType)) + + return mimeType == "application/json" || + strings.HasSuffix(mimeType, "+json") || + mimeType == "text/json" +} + func cdpHeaders(driverOptions scraperDriverOptions) map[string]interface{} { headers := map[string]interface{}{} if driverOptions.Headers != nil { diff --git a/pkg/scraper/url_test.go b/pkg/scraper/url_test.go new file mode 100644 index 0000000000..d75673b6d6 --- /dev/null +++ b/pkg/scraper/url_test.go @@ -0,0 +1,33 @@ +package scraper + +import "testing" + +func TestIsJSONMimeType(t *testing.T) { + tests := []struct { + name string + mimeType string + want bool + }{ + {"plain json", "application/json", true}, + {"json with charset", "application/json; charset=utf-8", true}, + {"json with charset and spacing", "application/json; charset=UTF-8", true}, + {"structured syntax suffix", "application/ld+json", true}, + {"uppercase", "APPLICATION/JSON", true}, + {"text/json", "text/json", true}, + {"html", "text/html", false}, + {"html with charset", "text/html; charset=utf-8", false}, + {"plain text", "text/plain", false}, + {"empty", "", false}, + {"contains json as substring but isn't json", "application/jsonp", false}, + {"contains json as substring but isn't json 2", "multipart/json-form-data", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := isJSONMimeType(tt.mimeType) + if got != tt.want { + t.Errorf("isJSONMimeType(%q) = %v, want %v", tt.mimeType, got, tt.want) + } + }) + } +} From 253f27ec947ae06f648011bad451d94bbee7c8c2 Mon Sep 17 00:00:00 2001 From: wraithfive Date: Sun, 2 Aug 2026 12:11:23 -0700 Subject: [PATCH 2/5] Address review feedback: fix data race, condense condition, warn on fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Guard jsonDoc's requestID/isJSON fields with a mutex — they were written from chromedp's event-processing goroutine and read from the action sequence without synchronization. - Condense the nested resource-type/mime-type checks into a single condition. - Log the OuterHTML fallback (when the CDP response body is no longer available) at Warn instead of Debug so it's visible without verbose logging. --- pkg/scraper/url.go | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/pkg/scraper/url.go b/pkg/scraper/url.go index c201e84cd2..0cdda1f3f5 100644 --- a/pkg/scraper/url.go +++ b/pkg/scraper/url.go @@ -11,6 +11,7 @@ import ( "os" "regexp" "strings" + "sync" "time" "github.com/chromedp/cdproto/cdp" @@ -259,15 +260,21 @@ func urlFromCDP(ctx context.Context, urlCDP string, driverOptions scraperDriverO // responses in an HTML pretty-printer when navigated to directly, so // OuterHTML on such a page returns HTML containing the JSON rather than // the JSON itself. - var jsonRequestID network.RequestID - var isJSONDocument bool + // jsonDoc is written from chromedp's event-processing goroutine (in the + // ListenTarget callback below) and read from the action sequence passed + // to chromedp.Run, so access must be synchronized. + var jsonDoc struct { + sync.Mutex + requestID network.RequestID + isJSON bool + } chromedp.ListenTarget(ctx, func(ev interface{}) { if ev, ok := ev.(*network.EventResponseReceived); ok { - if ev.Type == network.ResourceTypeDocument && !isJSONDocument { - if isJSONMimeType(ev.Response.MimeType) { - jsonRequestID = ev.RequestID - isJSONDocument = true - } + jsonDoc.Lock() + defer jsonDoc.Unlock() + if ev.Type == network.ResourceTypeDocument && !jsonDoc.isJSON && isJSONMimeType(ev.Response.MimeType) { + jsonDoc.requestID = ev.RequestID + jsonDoc.isJSON = true } } }) @@ -314,12 +321,16 @@ func urlFromCDP(ctx context.Context, urlCDP string, driverOptions scraperDriverO chromedp.Sleep(sleepDuration), setCDPClicks(driverOptions), chromedp.ActionFunc(func(ctx context.Context) error { - if isJSONDocument { - body, err := network.GetResponseBody(jsonRequestID).Do(ctx) + jsonDoc.Lock() + isJSON, requestID := jsonDoc.isJSON, jsonDoc.requestID + jsonDoc.Unlock() + + if isJSON { + body, err := network.GetResponseBody(requestID).Do(ctx) if err != nil { // fall back to OuterHTML if the response body is no // longer available (e.g. evicted from Chrome's cache) - logger.Debugf("[scraper] could not get raw response body for JSON document, falling back to OuterHTML: %v", err) + logger.Warnf("[scraper] could not get raw response body for JSON document, falling back to OuterHTML: %v", err) return chromedp.OuterHTML("html", &res, chromedp.ByQuery).Do(ctx) } res = string(body) From fec26b9f8ab4d79774745f6ad5eee188edbd6cb8 Mon Sep 17 00:00:00 2001 From: wraithfive Date: Sun, 2 Aug 2026 12:26:19 -0700 Subject: [PATCH 3/5] Use a named mutex field, matching project convention Every other hand-written mutex in this codebase uses a named field (e.g. mutex sync.Mutex) rather than an embedded one, so switch to match rather than promoting Lock/Unlock onto jsonDoc itself. --- pkg/scraper/url.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/scraper/url.go b/pkg/scraper/url.go index 0cdda1f3f5..08753def3f 100644 --- a/pkg/scraper/url.go +++ b/pkg/scraper/url.go @@ -264,14 +264,14 @@ func urlFromCDP(ctx context.Context, urlCDP string, driverOptions scraperDriverO // ListenTarget callback below) and read from the action sequence passed // to chromedp.Run, so access must be synchronized. var jsonDoc struct { - sync.Mutex + mutex sync.Mutex requestID network.RequestID isJSON bool } chromedp.ListenTarget(ctx, func(ev interface{}) { if ev, ok := ev.(*network.EventResponseReceived); ok { - jsonDoc.Lock() - defer jsonDoc.Unlock() + jsonDoc.mutex.Lock() + defer jsonDoc.mutex.Unlock() if ev.Type == network.ResourceTypeDocument && !jsonDoc.isJSON && isJSONMimeType(ev.Response.MimeType) { jsonDoc.requestID = ev.RequestID jsonDoc.isJSON = true @@ -321,9 +321,9 @@ func urlFromCDP(ctx context.Context, urlCDP string, driverOptions scraperDriverO chromedp.Sleep(sleepDuration), setCDPClicks(driverOptions), chromedp.ActionFunc(func(ctx context.Context) error { - jsonDoc.Lock() + jsonDoc.mutex.Lock() isJSON, requestID := jsonDoc.isJSON, jsonDoc.requestID - jsonDoc.Unlock() + jsonDoc.mutex.Unlock() if isJSON { body, err := network.GetResponseBody(requestID).Do(ctx) From e19b2aa7d6ed28eeadec662d5634edc02742d7ed Mon Sep 17 00:00:00 2001 From: wraithfive Date: Sun, 2 Aug 2026 12:54:12 -0700 Subject: [PATCH 4/5] Add a unit test that catches the CDP JSON-document race Extract the racy state (requestID/isJSON) out of urlFromCDP into a small named type, jsonDocumentTracker, with markJSON/get methods guarding access with its mutex. This makes the concurrency behavior independently testable without spinning up chromedp/a real browser. TestJSONDocumentTrackerConcurrentAccess exercises markJSON and get from separate goroutines concurrently and repeatedly, matching how urlFromCDP actually uses it (one goroutine from chromedp's event-processing loop, one from the action sequence passed to chromedp.Run). Verified this test reliably fails under `go test -race` if the mutex is removed, and passes cleanly with it in place. Also adds TestJSONDocumentTrackerMarksOnlyFirstJSONMatch to cover the "only track the first JSON document" behavior directly. --- pkg/scraper/url.go | 54 +++++++++++++++++++++++----------- pkg/scraper/url_test.go | 64 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 101 insertions(+), 17 deletions(-) diff --git a/pkg/scraper/url.go b/pkg/scraper/url.go index 08753def3f..063ab526d1 100644 --- a/pkg/scraper/url.go +++ b/pkg/scraper/url.go @@ -260,21 +260,11 @@ func urlFromCDP(ctx context.Context, urlCDP string, driverOptions scraperDriverO // responses in an HTML pretty-printer when navigated to directly, so // OuterHTML on such a page returns HTML containing the JSON rather than // the JSON itself. - // jsonDoc is written from chromedp's event-processing goroutine (in the - // ListenTarget callback below) and read from the action sequence passed - // to chromedp.Run, so access must be synchronized. - var jsonDoc struct { - mutex sync.Mutex - requestID network.RequestID - isJSON bool - } + var jsonDoc jsonDocumentTracker chromedp.ListenTarget(ctx, func(ev interface{}) { if ev, ok := ev.(*network.EventResponseReceived); ok { - jsonDoc.mutex.Lock() - defer jsonDoc.mutex.Unlock() - if ev.Type == network.ResourceTypeDocument && !jsonDoc.isJSON && isJSONMimeType(ev.Response.MimeType) { - jsonDoc.requestID = ev.RequestID - jsonDoc.isJSON = true + if ev.Type == network.ResourceTypeDocument { + jsonDoc.markJSON(ev.RequestID, ev.Response.MimeType) } } }) @@ -321,9 +311,7 @@ func urlFromCDP(ctx context.Context, urlCDP string, driverOptions scraperDriverO chromedp.Sleep(sleepDuration), setCDPClicks(driverOptions), chromedp.ActionFunc(func(ctx context.Context) error { - jsonDoc.mutex.Lock() - isJSON, requestID := jsonDoc.isJSON, jsonDoc.requestID - jsonDoc.mutex.Unlock() + requestID, isJSON := jsonDoc.get() if isJSON { body, err := network.GetResponseBody(requestID).Do(ctx) @@ -418,6 +406,40 @@ func isJSONMimeType(mimeType string) bool { mimeType == "text/json" } +// jsonDocumentTracker records whether the CDP-loaded page's main document +// turned out to be JSON, and if so, which network request to fetch its raw +// body from. It's written from chromedp's event-processing goroutine (via +// markJSON, called from a ListenTarget callback) and read from the action +// sequence passed to chromedp.Run (via get), so access is synchronized with +// a mutex. +type jsonDocumentTracker struct { + mutex sync.Mutex + requestID network.RequestID + isJSON bool +} + +// markJSON records requestID as the main JSON document if mimeType +// indicates JSON content and no JSON document has been recorded yet. +// Subsequent calls after the first match are no-ops. +func (t *jsonDocumentTracker) markJSON(requestID network.RequestID, mimeType string) { + t.mutex.Lock() + defer t.mutex.Unlock() + + if !t.isJSON && isJSONMimeType(mimeType) { + t.requestID = requestID + t.isJSON = true + } +} + +// get returns the recorded JSON request ID and whether a JSON document has +// been found. +func (t *jsonDocumentTracker) get() (network.RequestID, bool) { + t.mutex.Lock() + defer t.mutex.Unlock() + + return t.requestID, t.isJSON +} + func cdpHeaders(driverOptions scraperDriverOptions) map[string]interface{} { headers := map[string]interface{}{} if driverOptions.Headers != nil { diff --git a/pkg/scraper/url_test.go b/pkg/scraper/url_test.go index d75673b6d6..4f53ef7cd0 100644 --- a/pkg/scraper/url_test.go +++ b/pkg/scraper/url_test.go @@ -1,6 +1,12 @@ package scraper -import "testing" +import ( + "fmt" + "sync" + "testing" + + "github.com/chromedp/cdproto/network" +) func TestIsJSONMimeType(t *testing.T) { tests := []struct { @@ -31,3 +37,59 @@ func TestIsJSONMimeType(t *testing.T) { }) } } + +func TestJSONDocumentTrackerMarksOnlyFirstJSONMatch(t *testing.T) { + var tracker jsonDocumentTracker + + tracker.markJSON("req-1", "text/html") + if _, isJSON := tracker.get(); isJSON { + t.Fatal("non-JSON mime type should not be recorded") + } + + tracker.markJSON("req-2", "application/json") + if requestID, isJSON := tracker.get(); !isJSON || requestID != "req-2" { + t.Fatalf("get() = (%q, %v), want (%q, true)", requestID, isJSON, "req-2") + } + + tracker.markJSON("req-3", "application/json") + if requestID, isJSON := tracker.get(); !isJSON || requestID != "req-2" { + t.Fatalf("a second JSON match overwrote the first: get() = (%q, %v), want (%q, true)", requestID, isJSON, "req-2") + } +} + +// TestJSONDocumentTrackerConcurrentAccess exercises jsonDocumentTracker the +// way urlFromCDP actually uses it: one goroutine (standing in for chromedp's +// event-processing goroutine) calls markJSON while another goroutine +// (standing in for the action sequence passed to chromedp.Run) calls get, +// concurrently and repeatedly. Before requestID/isJSON were guarded by a +// mutex, `go test -race` reliably flagged this exact access pattern as a +// data race. +func TestJSONDocumentTrackerConcurrentAccess(t *testing.T) { + var tracker jsonDocumentTracker + + const n = 200 + var wg sync.WaitGroup + wg.Add(2 * n) + + for i := 0; i < n; i++ { + i := i + go func() { + defer wg.Done() + tracker.markJSON(network.RequestID(fmt.Sprintf("req-%d", i)), "application/json") + }() + go func() { + defer wg.Done() + tracker.get() + }() + } + + wg.Wait() + + requestID, isJSON := tracker.get() + if !isJSON { + t.Fatal("expected a JSON document to have been recorded") + } + if requestID == "" { + t.Fatal("expected a non-empty request ID to have been recorded") + } +} From 130c0df91ec3ed775429567b78d43634e7849e49 Mon Sep 17 00:00:00 2001 From: wraithfive Date: Sun, 2 Aug 2026 13:26:31 -0700 Subject: [PATCH 5/5] Fix iframe JSON responses hijacking extraction for HTML CDP scrapers network.ResourceTypeDocument fires for iframe navigations too, not just the top-level one. The tracker previously latched onto the first Document response that happened to be JSON, so an HTML scraper whose page embeds an iframe that loads JSON (an embed widget, an ad frame) could have its extraction hijacked by that iframe's response instead of the page's own HTML - a regression risk for the population this fix was never meant to touch. Since redirects don't produce a separate responseReceived event for the pre-redirect URL (that arrives via requestWillBeSent's redirectResponse on the same request instead), the first Document response chromedp sees reliably corresponds to the top-level navigation. So rather than plumbing through frame IDs, jsonDocumentTracker now records the *first* Document response unconditionally - JSON or not - and ignores every one after it. Renamed markJSON/get to markDocument/mainDocument to match the new semantics, and reworded the OuterHTML-fallback log line to say the scrape will likely still fail as a result, so it doesn't read as an unrelated warning next to the downstream "not valid json" error. TestJSONDocumentTrackerOnlyRecordsFirstDocument replaces the old first-JSON-match test with two cases, including the exact iframe scenario above; verified it fails under the previous logic and passes with this one. --- pkg/scraper/url.go | 61 ++++++++++++++++++++++++++++------------- pkg/scraper/url_test.go | 61 +++++++++++++++++++++++++---------------- 2 files changed, 79 insertions(+), 43 deletions(-) diff --git a/pkg/scraper/url.go b/pkg/scraper/url.go index 063ab526d1..8441bc4f83 100644 --- a/pkg/scraper/url.go +++ b/pkg/scraper/url.go @@ -264,7 +264,7 @@ func urlFromCDP(ctx context.Context, urlCDP string, driverOptions scraperDriverO chromedp.ListenTarget(ctx, func(ev interface{}) { if ev, ok := ev.(*network.EventResponseReceived); ok { if ev.Type == network.ResourceTypeDocument { - jsonDoc.markJSON(ev.RequestID, ev.Response.MimeType) + jsonDoc.markDocument(ev.RequestID, ev.Response.MimeType) } } }) @@ -311,14 +311,19 @@ func urlFromCDP(ctx context.Context, urlCDP string, driverOptions scraperDriverO chromedp.Sleep(sleepDuration), setCDPClicks(driverOptions), chromedp.ActionFunc(func(ctx context.Context) error { - requestID, isJSON := jsonDoc.get() + requestID, isJSON := jsonDoc.mainDocument() if isJSON { body, err := network.GetResponseBody(requestID).Do(ctx) if err != nil { - // fall back to OuterHTML if the response body is no - // longer available (e.g. evicted from Chrome's cache) - logger.Warnf("[scraper] could not get raw response body for JSON document, falling back to OuterHTML: %v", err) + // Fall back to OuterHTML if the response body is no + // longer available (e.g. evicted from Chrome's cache). + // The scraper will most likely still fail downstream on + // this fallback, since OuterHTML returns Chrome's + // HTML-wrapped view of the JSON rather than the JSON + // itself - but it's a better failure mode than losing + // the response entirely. + logger.Warnf("[scraper] could not get raw response body for JSON document, falling back to OuterHTML (the scrape will likely still fail as a result): %v", err) return chromedp.OuterHTML("html", &res, chromedp.ByQuery).Do(ctx) } res = string(body) @@ -408,32 +413,50 @@ func isJSONMimeType(mimeType string) bool { // jsonDocumentTracker records whether the CDP-loaded page's main document // turned out to be JSON, and if so, which network request to fetch its raw -// body from. It's written from chromedp's event-processing goroutine (via -// markJSON, called from a ListenTarget callback) and read from the action -// sequence passed to chromedp.Run (via get), so access is synchronized with -// a mutex. +// body from. +// +// It only ever considers the *first* Document-type response it sees, via +// markDocument. Network.responseReceived fires with resourceType Document +// for iframe navigations too, not just the top-level one, so a naive +// "first Document that happens to be JSON" rule could latch onto an +// iframe's JSON response instead of the main page's HTML - misidentifying +// an HTML scrape as a JSON one. Redirects don't complicate this: a +// pre-redirect response never gets its own responseReceived event (that +// history arrives via requestWillBeSent's redirectResponse on the same +// request instead), so the first Document response reliably corresponds to +// the top-level navigation. A click-triggered navigation (via +// setCDPClicks, which runs after the initial Navigate) is not treated as a +// new "first" document either, so if a click leads to a JSON page, this +// still reports the original navigation's result. +// +// It's written from chromedp's event-processing goroutine (via +// markDocument, called from a ListenTarget callback) and read from the +// action sequence passed to chromedp.Run (via mainDocument), so access is +// synchronized with a mutex. type jsonDocumentTracker struct { mutex sync.Mutex requestID network.RequestID isJSON bool + recorded bool } -// markJSON records requestID as the main JSON document if mimeType -// indicates JSON content and no JSON document has been recorded yet. -// Subsequent calls after the first match are no-ops. -func (t *jsonDocumentTracker) markJSON(requestID network.RequestID, mimeType string) { +// markDocument records requestID as the main document if no document has +// been recorded yet. Subsequent calls after the first are no-ops. +func (t *jsonDocumentTracker) markDocument(requestID network.RequestID, mimeType string) { t.mutex.Lock() defer t.mutex.Unlock() - if !t.isJSON && isJSONMimeType(mimeType) { - t.requestID = requestID - t.isJSON = true + if t.recorded { + return } + t.recorded = true + t.requestID = requestID + t.isJSON = isJSONMimeType(mimeType) } -// get returns the recorded JSON request ID and whether a JSON document has -// been found. -func (t *jsonDocumentTracker) get() (network.RequestID, bool) { +// mainDocument returns the main document's request ID and whether it was +// JSON. +func (t *jsonDocumentTracker) mainDocument() (network.RequestID, bool) { t.mutex.Lock() defer t.mutex.Unlock() diff --git a/pkg/scraper/url_test.go b/pkg/scraper/url_test.go index 4f53ef7cd0..9f84e24b0f 100644 --- a/pkg/scraper/url_test.go +++ b/pkg/scraper/url_test.go @@ -38,32 +38,46 @@ func TestIsJSONMimeType(t *testing.T) { } } -func TestJSONDocumentTrackerMarksOnlyFirstJSONMatch(t *testing.T) { - var tracker jsonDocumentTracker +func TestJSONDocumentTrackerOnlyRecordsFirstDocument(t *testing.T) { + t.Run("first document is JSON, later ones don't override it", func(t *testing.T) { + var tracker jsonDocumentTracker - tracker.markJSON("req-1", "text/html") - if _, isJSON := tracker.get(); isJSON { - t.Fatal("non-JSON mime type should not be recorded") - } + tracker.markDocument("req-1", "application/json") + tracker.markDocument("req-2", "application/json") + tracker.markDocument("req-3", "text/html") - tracker.markJSON("req-2", "application/json") - if requestID, isJSON := tracker.get(); !isJSON || requestID != "req-2" { - t.Fatalf("get() = (%q, %v), want (%q, true)", requestID, isJSON, "req-2") - } + if requestID, isJSON := tracker.mainDocument(); !isJSON || requestID != "req-1" { + t.Fatalf("mainDocument() = (%q, %v), want (%q, true)", requestID, isJSON, "req-1") + } + }) - tracker.markJSON("req-3", "application/json") - if requestID, isJSON := tracker.get(); !isJSON || requestID != "req-2" { - t.Fatalf("a second JSON match overwrote the first: get() = (%q, %v), want (%q, true)", requestID, isJSON, "req-2") - } + t.Run("first document is HTML, a later JSON response doesn't override it", func(t *testing.T) { + // This is the main-frame-vs-iframe scenario: an HTML scrape whose + // page contains an iframe that loads JSON (an embed widget, an ad + // frame) also fires a Document-type responseReceived for that + // iframe. Since the top-level HTML document is always the first + // Document response chromedp sees for a given navigation, the + // tracker must not let this later JSON response override it - + // otherwise an HTML scraper would incorrectly receive the iframe's + // JSON body instead of the page's HTML. + var tracker jsonDocumentTracker + + tracker.markDocument("req-main-page", "text/html") + tracker.markDocument("req-iframe", "application/json") + + if requestID, isJSON := tracker.mainDocument(); isJSON || requestID != "req-main-page" { + t.Fatalf("mainDocument() = (%q, %v), want (%q, false)", requestID, isJSON, "req-main-page") + } + }) } // TestJSONDocumentTrackerConcurrentAccess exercises jsonDocumentTracker the // way urlFromCDP actually uses it: one goroutine (standing in for chromedp's -// event-processing goroutine) calls markJSON while another goroutine -// (standing in for the action sequence passed to chromedp.Run) calls get, -// concurrently and repeatedly. Before requestID/isJSON were guarded by a -// mutex, `go test -race` reliably flagged this exact access pattern as a -// data race. +// event-processing goroutine) calls markDocument while another goroutine +// (standing in for the action sequence passed to chromedp.Run) calls +// mainDocument, concurrently and repeatedly. Before requestID/isJSON/recorded +// were guarded by a mutex, `go test -race` reliably flagged this exact +// access pattern as a data race. func TestJSONDocumentTrackerConcurrentAccess(t *testing.T) { var tracker jsonDocumentTracker @@ -71,21 +85,20 @@ func TestJSONDocumentTrackerConcurrentAccess(t *testing.T) { var wg sync.WaitGroup wg.Add(2 * n) - for i := 0; i < n; i++ { - i := i + for i := range n { go func() { defer wg.Done() - tracker.markJSON(network.RequestID(fmt.Sprintf("req-%d", i)), "application/json") + tracker.markDocument(network.RequestID(fmt.Sprintf("req-%d", i)), "application/json") }() go func() { defer wg.Done() - tracker.get() + tracker.mainDocument() }() } wg.Wait() - requestID, isJSON := tracker.get() + requestID, isJSON := tracker.mainDocument() if !isJSON { t.Fatal("expected a JSON document to have been recorded") }