Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 28 additions & 5 deletions colly.go
Original file line number Diff line number Diff line change
Expand Up @@ -1417,10 +1417,13 @@ func (c *Collector) Cookies(URL string) []*http.Cookie {
}

// Clone creates an exact copy of a Collector without callbacks.
// HTTP backend, robots.txt cache and cookie jar are shared
// between collectors.
// The visited-URL store, robots.txt cache, cookie jar and HTTP transport
// are shared between collectors so cookies and the connection pool flow
// across them. The *http.Client and the callback-protecting lock are
// independent so per-collector settings like AllowURLRevisit take effect
// on the clone's redirects without leaking back into the parent.
func (c *Collector) Clone() *Collector {
return &Collector{
clone := &Collector{
AllowedDomains: c.AllowedDomains,
AllowURLRevisit: c.AllowURLRevisit,
CacheDir: c.CacheDir,
Expand All @@ -1441,20 +1444,40 @@ func (c *Collector) Clone() *Collector {
TraceHTTP: c.TraceHTTP,
Context: c.Context,
store: c.store,
backend: c.backend,
debugger: c.debugger,
Async: c.Async,
redirectHandler: c.redirectHandler,
errorCallbacks: make([]ErrorCallback, 0, 8),
htmlCallbacks: make([]*htmlCallbackContainer, 0, 8),
xmlCallbacks: make([]*xmlCallbackContainer, 0, 8),
scrapedCallbacks: make([]ScrapedCallback, 0, 8),
lock: c.lock,
lock: &sync.RWMutex{},
requestCallbacks: make([]RequestCallback, 0, 8),
responseCallbacks: make([]ResponseCallback, 0, 8),
robotsMap: c.robotsMap,
wg: &sync.WaitGroup{},
}

// Independent httpBackend + http.Client so CheckRedirect closes over
// the clone, not the parent. Without this, the redirect handler would
// evaluate the parent's AllowURLRevisit and filters, so toggling those
// on the clone has no effect on redirects.
//
// Transport, cookie jar and Timeout are kept shared so connection
// pooling and session cookies continue to flow between collectors.
// LimitRules are copied by value (the slice header) — each *LimitRule
// is still shared because LimitRule.Init is idempotent.
clone.backend = &httpBackend{
LimitRules: append([]*LimitRule(nil), c.backend.LimitRules...),
lock: &sync.RWMutex{},
Client: &http.Client{
Transport: c.backend.Client.Transport,
Jar: c.backend.Client.Jar,
Timeout: c.backend.Client.Timeout,
CheckRedirect: clone.checkRedirectFunc(),
},
}
return clone
}

func (c *Collector) checkRedirectFunc() func(req *http.Request, via []*http.Request) error {
Expand Down
49 changes: 49 additions & 0 deletions colly_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2236,3 +2236,52 @@ func TestLimitRuleClone(t *testing.T) {
t.Error("clone.Init() must not mutate the source's unexported state")
}
}

// TestCloneAllowURLRevisitIndependent verifies that AllowURLRevisit on a
// cloned Collector takes effect during redirects, independently of the
// parent's setting.
//
// Previously Clone() reused the parent's *http.Client, including its
// CheckRedirect closure — and that closure captures the *parent*
// Collector. As a result, the clone's CheckRedirect evaluated the
// parent's AllowURLRevisit field, so toggling AllowURLRevisit on the
// clone had no effect on redirects: visiting a redirect endpoint twice
// failed with "already visited" even though the clone allowed revisits.
func TestCloneAllowURLRevisitIndependent(t *testing.T) {
ts := newTestServer()
defer ts.Close()

parent := NewCollector()
clone := parent.Clone()
clone.AllowURLRevisit = true

redirectURL := ts.URL + "/redirect"
finalURL := ts.URL + "/redirected/"

requestCount := make(map[string]int)
clone.OnRequest(func(r *Request) {
requestCount[r.URL.String()]++
})
responseCount := make(map[string]int)
clone.OnResponse(func(r *Response) {
responseCount[r.Request.URL.String()]++
})

for i := 0; i < 2; i++ {
if err := clone.Visit(redirectURL); err != nil {
t.Fatalf("visit %d: unexpected error: %v", i+1, err)
}
}
if requestCount[redirectURL] != 2 {
t.Errorf("redirect URL visited %d times, want 2", requestCount[redirectURL])
}
if responseCount[finalURL] != 2 {
t.Errorf("final URL visited %d times, want 2", responseCount[finalURL])
}

// Parent should keep its original (default false) AllowURLRevisit and
// not gain revisit semantics just because the clone toggled it.
if parent.AllowURLRevisit {
t.Error("parent.AllowURLRevisit was mutated by setting it on the clone")
}
}
Loading