diff --git a/colly.go b/colly.go index f69cfb5e..fd2c2849 100644 --- a/colly.go +++ b/colly.go @@ -676,10 +676,10 @@ func (c *Collector) scrape(u, method string, depth int, requestData io.Reader, c // replace this with http.NewRequestWithContext req = req.WithContext(context.WithValue(c.Context, CheckRevisitKey, checkRevisit)) - if err := c.requestCheck(parsedURL, method, req.GetBody, depth, checkRevisit); err != nil { + u = parsedURL.String() + if err := c.requestCheck(parsedURL, u, method, req.GetBody, depth, checkRevisit); err != nil { return err } - u = parsedURL.String() c.wg.Add(1) if c.Async { go c.fetch(u, method, depth, requestData, ctx, hdr, req) @@ -771,8 +771,7 @@ func (c *Collector) fetch(u, method string, depth int, requestData io.Reader, ct return err } -func (c *Collector) requestCheck(parsedURL *url.URL, method string, getBody func() (io.ReadCloser, error), depth int, checkRevisit bool) error { - u := parsedURL.String() +func (c *Collector) requestCheck(parsedURL *url.URL, u, method string, getBody func() (io.ReadCloser, error), depth int, checkRevisit bool) error { if c.MaxDepth > 0 && c.MaxDepth < depth { return ErrMaxDepth } @@ -804,7 +803,7 @@ func (c *Collector) requestCheck(parsedURL *url.URL, method string, getBody func } defer body.Close() } - uHash := requestHash(u, body) + uHash := requestHashNormalized(u, body) visited, err := c.store.IsVisited(uHash) if err != nil { return err @@ -1493,7 +1492,7 @@ func (c *Collector) checkRedirectFunc() func(req *http.Request, via []*http.Requ } defer body.Close() } - uHash := requestHash(req.URL.String(), body) + uHash := requestHashNormalized(normalizedURL, body) visited, err := c.store.IsVisited(uHash) if err != nil { return err @@ -1667,10 +1666,14 @@ func normalizeURL(u string) string { } func requestHash(url string, body io.Reader) uint64 { - h := fnv.New64a() // reparse the url to fix ambiguities such as // "http://example.com" vs "http://example.com/" - io.WriteString(h, normalizeURL(url)) + return requestHashNormalized(normalizeURL(url), body) +} + +func requestHashNormalized(url string, body io.Reader) uint64 { + h := fnv.New64a() + io.WriteString(h, url) if body != nil { io.Copy(h, body) } diff --git a/colly_test.go b/colly_test.go index 06a7b1dd..d666a882 100644 --- a/colly_test.go +++ b/colly_test.go @@ -1888,6 +1888,89 @@ func BenchmarkOnResponse(b *testing.B) { } } +func TestRequestHashNormalizedMatchesRequestHash(t *testing.T) { + tests := []struct { + name string + raw string + body string + }{ + { + name: "adds trailing slash", + raw: "https://example.com", + }, + { + name: "keeps path and query", + raw: "https://example.com/articles/12345?category=scraping", + }, + { + name: "normalizes single percent sign", + raw: "https://example.com/100%", + }, + { + name: "preserves post body", + raw: "https://example.com/login", + body: "name=colly", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + parsedWhatwgURL, err := urlParser.Parse(tt.raw) + if err != nil { + t.Fatal(err) + } + parsedURL, err := url.Parse(parsedWhatwgURL.Href(false)) + if err != nil { + t.Fatal(err) + } + + got := requestHashNormalized(parsedURL.String(), strings.NewReader(tt.body)) + want := requestHash(tt.raw, strings.NewReader(tt.body)) + if got != want { + t.Fatalf("requestHashNormalized() = %d, want %d", got, want) + } + }) + } +} + +type benchmarkStorage struct{} + +func (benchmarkStorage) Init() error { + return nil +} + +func (benchmarkStorage) Visited(uint64) error { + return nil +} + +func (benchmarkStorage) IsVisited(uint64) (bool, error) { + return false, nil +} + +func (benchmarkStorage) Cookies(*url.URL) string { + return "" +} + +func (benchmarkStorage) SetCookies(*url.URL, string) {} + +func BenchmarkRequestCheck(b *testing.B) { + c := NewCollector() + c.store = benchmarkStorage{} + + u, err := url.Parse("https://example.com/articles/12345?category=scraping") + if err != nil { + b.Fatal(err) + } + urlString := u.String() + + b.ReportAllocs() + for n := 0; n < b.N; n++ { + if err := c.requestCheck(u, urlString, "GET", nil, 1, true); err != nil { + b.Fatal(err) + } + } +} + func requireSessionCookieSimple(handler http.Handler) http.Handler { const cookieName = "session_id"