Skip to content

fix:The rl,rls options are not supported #1434#1731

Open
falcolnic wants to merge 3 commits intoprojectdiscovery:devfrom
falcolnic:fix-rl-rls
Open

fix:The rl,rls options are not supported #1434#1731
falcolnic wants to merge 3 commits intoprojectdiscovery:devfrom
falcolnic:fix-rl-rls

Conversation

@falcolnic
Copy link

@falcolnic falcolnic commented Feb 4, 2026

Proposed changes

/claim #1434
Fixed a bug in the global rate limit (-rl) implementation where it was not applied to sources without explicit per-source rate limits (-rls).

Proof

Before fix (v2.6.7) — all domains throttled to ~30s:
image

After fix (v2.12.0) — fast domains complete quickly, rate limit respected:
image(1)
Also added passive_test.go

Checklist

  • Pull request is created against the dev branch
  • All checks passed (lint, unit/integration/regression tests etc.) with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Summary by CodeRabbit

  • New Features
    • Per-source rate limits now support custom durations and max counts, with computed duration (not a fixed value) and automatic fallback to the global rate limit when absent.
  • Bug Fixes
    • Negative global rate limits are clamped to zero; unlimited/very-high limits keep prior behavior.
  • Tests
    • Added tests covering per-source custom limits, duration handling, fallback, and global rate-limit scenarios.

@coderabbitai
Copy link

coderabbitai bot commented Feb 4, 2026

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title refers to fixing rate limit options (rl, rls) which is directly addressed in the changes that implement per-source rate limit handling and global rate limit application.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@pkg/passive/passive.go`:
- Around line 85-105: The buildMultiRateLimiter function currently dereferences
rateLimit.Custom and can panic when rateLimit is nil; update
buildMultiRateLimiter (method on Agent) to first check for nil rateLimit and nil
rateLimit.Custom (e.g., create a local safe reference or treat it as empty)
before calling rateLimit.Custom.Get(strings.ToLower(source.Name())); if nil,
always fall back to the globalRateLimit path so per-source lookup is skipped;
ensure EnumerateSubdomainsWithCtx's nil customRateLimiter case is handled safely
by this guard.
🧹 Nitpick comments (1)
pkg/passive/passive_test.go (1)

34-96: Add a subtest for negative global rate limits.
This exercises the new clamp logic and guards against regression.

✅ Suggested subtest
 	t.Run("GlobalRateLimitAppliedWhenSourceNotInCustom", func(t *testing.T) {
 		agent := New([]string{"crtsh", "hackertarget"}, []string{}, false, false)
 		customRL := &subscraping.CustomRateLimit{
 			Custom: mapsutil.SyncLockMap[string, subscraping.RateLimitSpec]{Map: make(map[string]subscraping.RateLimitSpec)},
 		}
@@
 		require.NoError(t, err)
 		require.NotNil(t, multiRateLimiter)
 	})
+
+	t.Run("NegativeGlobalRateLimitClampsToZero", func(t *testing.T) {
+		agent := New([]string{"crtsh"}, []string{}, false, false)
+		customRL := &subscraping.CustomRateLimit{
+			Custom: mapsutil.SyncLockMap[string, subscraping.RateLimitSpec]{Map: make(map[string]subscraping.RateLimitSpec)},
+		}
+
+		multiRateLimiter, err := agent.buildMultiRateLimiter(ctx, -5, customRL)
+
+		require.NoError(t, err)
+		require.NotNil(t, multiRateLimiter)
+	})

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@pkg/passive/passive.go`:
- Around line 97-110: The indentation in the if-block that handles rateLimit
(the block checking "if rateLimit != nil" where sourceRateLimit is retrieved and
rl/duration/globalRateLimit are assigned) uses spaces instead of tabs; run gofmt
or goimports on pkg/passive/passive.go (or manually re-indent that block) to
convert those leading spaces to tabs so the file matches Go formatting
conventions and linter expectations.

@falcolnic
Copy link
Author

@dogancanbakir Hey, any news?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant