fix: return HTML for browser Accept headers on root route (#532)#546
fix: return HTML for browser Accept headers on root route (#532)#546Justxd22 wants to merge 7 commits intocameri:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes / content negotiation so NIP-11 relay information is only served when the client explicitly requests application/nostr+json, preventing typical browser Accept headers from receiving JSON.
Changes:
- Replaced
accepts-based negotiation with an explicitAccept: application/nostr+json(andq>0) check shared between router middleware androotRequestHandler. - Added unit tests for the explicit Accept header detection (including browser-style headers and
q=0). - Added an integration scenario ensuring browser-style Accept headers receive HTML on
/.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/handlers/request-handlers/root-request-handler.spec.ts | Adds unit coverage for explicit NIP-11 Accept detection and related root handler behavior. |
| test/integration/features/nip-11/nip-11.feature.ts | Adds a Cucumber step that simulates a browser-like root request. |
| test/integration/features/nip-11/nip-11.feature | Adds an integration scenario asserting HTML is served for browser-style Accept headers. |
| src/routes/index.ts | Switches router middleware to use the explicit Accept-header check instead of accepts. |
| src/handlers/request-handlers/root-request-handler.ts | Introduces hasExplicitNostrJsonAcceptHeader and uses it for root handler content negotiation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Ci should pass once #539 is merged |
|
Did you test the fix manually? Also does it work when the relay URL is in a path other than /? |
|
@cameri confirmed it breaks under subpath! so We need to edit the html templates remove hardcoded root url and inject |
🦋 Changeset detectedLatest commit: a3224f8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@cameri ready ! |
Description
/content negotiation to serve NIP-11 only for explicitapplication/nostr+jsonrequestsAcceptheaders (e.g.text/html,...,*/*) from being treated as NIP-11 requestsrootRequestHandlerq=0Related Issue
#532
How Has This Been Tested?
npm run build:checknpm run test:unitTypes of changes
Checklist: