-
Notifications
You must be signed in to change notification settings - Fork 37k
Allow redirects to trusted domains #284438
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds functionality to allow redirects to trusted domains in the web content extractor service. The changes enable the WebPageLoader to accept a callback for checking trusted domains and to automatically allow redirects between www and non-www versions of the same domain.
Key Changes:
- Modified redirect logic to normalize domain authorities (removing "www." prefix for comparison)
- Added a trusted domain callback parameter to WebPageLoader constructor
- Removed the
equalsIgnoreCaseimport as it's no longer needed with the new normalization approach - Added comprehensive test coverage for www/non-www redirects and trusted domain validation
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/vs/platform/webContentExtractor/electron-main/webPageLoader.ts |
Refactored redirect logic to add www-prefix normalization and trusted domain checking via callback |
src/vs/platform/webContentExtractor/test/electron-main/webPageLoader.test.ts |
Added 5 new test cases covering www redirects, trusted domains, non-trusted domains, and wildcard subdomain matching |
src/vs/platform/webContentExtractor/electron-main/webContentExtractorService.ts |
CRITICAL ISSUES: Contains commented-out import and attempts to inject ITrustedDomainService which causes compilation errors and violates layering architecture |
Critical Issues Identified:
The webContentExtractorService.ts file has compilation errors due to a commented-out import and attempts to use ITrustedDomainService from the workbench layer in the platform layer, which violates VS Code's strict layering architecture where platform cannot depend on workbench.
Fixes #278618 (no redirects in the chat)
Fixes #280714 (partial in that we will silently redirect for trusted domains, nytimes is not part of that)