Avoid redundant URL normalization in request checks

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)
 	}

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"