DRAFT: Amp 149016 default referrers fallback#1540
Open
daniel-graham-amplitude wants to merge 3 commits intoAMP-149016-cookie-enabled-reentrancy-bugfrom
Open
DRAFT: Amp 149016 default referrers fallback#1540daniel-graham-amplitude wants to merge 3 commits intoAMP-149016-cookie-enabled-reentrancy-bugfrom
daniel-graham-amplitude wants to merge 3 commits intoAMP-149016-cookie-enabled-reentrancy-bugfrom
Conversation
daniel-graham-amplitude
changed the base branch from
main
to
AMP-149016-cookie-enabled-reentrancy-bug
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| const parts = hostname.split('.'); | ||
| let tld = parts[parts.length - 1]; | ||
| let name = parts[parts.length - 2]; | ||
| if (KNOWN_2LDS.find((tld) => hostname.endsWith(`.${tld}`))) { |
There was a problem hiding this comment.
Callback parameter shadows outer variable tld
Low Severity
The find callback parameter tld on line 146 shadows the outer let tld declared on line 144. While the current logic works correctly (the outer tld is still properly reassigned in the if block body, outside the callback scope), this shadowing is confusing and fragile. A future developer modifying this code could easily misunderstand which tld is in scope. Additionally, Array.prototype.some would be more idiomatic here since the found value is only used as a boolean.
Additional Locations (1)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Checklist
Note
Medium Risk
Changes default referrer-exclusion behavior, which can affect campaign attribution/session logic across different hostnames and TLD edge cases; logic is bounded and covered by new tests but still heuristic.
Overview
When
getDefaultExcludedReferrers()is called without a configured cookie domain, it now derives a best-effort base domain fromlocation.hostname(including support for a curated set of common 2-level TLDs) and builds the exclusion regex from that value.This also tightens regex escaping (escapes all dots) and expands unit tests by mocking
global.locationand adding cases for 1-level domains, known 2-level TLDs, and edge cases likelocalhostand IP hostnames.Written by Cursor Bugbot for commit 7113edc. This will update automatically on new commits. Configure here.