Skip to content

fix: return HTML for browser Accept headers on root route (#532)#546

Open
Justxd22 wants to merge 4 commits intocameri:mainfrom
Justxd22:fix/532-root-browser-html
Open

fix: return HTML for browser Accept headers on root route (#532)#546
Justxd22 wants to merge 4 commits intocameri:mainfrom
Justxd22:fix/532-root-browser-html

Conversation

@Justxd22
Copy link
Copy Markdown
Collaborator

Description

  • fix / content negotiation to serve NIP-11 only for explicit application/nostr+json requests
  • prevent typical browser Accept headers (e.g. text/html,...,*/*) from being treated as NIP-11 requests
  • reuse the same explicit Accept check in router middleware and rootRequestHandler
  • add unit coverage for explicit NIP-11, browser Accept header, and q=0
  • add integration coverage for browser-style Accept header returning HTML

Related Issue

#532

How Has This Been Tested?

  • npm run build:check
  • npm run test:unit

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my code changes.
  • I added a changeset, or this is docs-only and I added an empty changeset.
  • All new and existing tests passed.

Copilot AI review requested due to automatic review settings April 19, 2026 23:15
Copy link
Copy Markdown

Copilot AI left a 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 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 explicit Accept: application/nostr+json (and q>0) check shared between router middleware and rootRequestHandler.
  • 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.

Comment thread src/routes/index.ts Outdated
Comment thread src/handlers/request-handlers/root-request-handler.ts Outdated
@Justxd22
Copy link
Copy Markdown
Collaborator Author

Ci should pass once #539 is merged

@cameri
Copy link
Copy Markdown
Owner

cameri commented Apr 20, 2026

Did you test the fix manually? Also does it work when the relay URL is in a path other than /?

@Justxd22
Copy link
Copy Markdown
Collaborator Author

@cameri confirmed it breaks under subpath! so We need to edit the html templates remove hardcoded root url and inject settings.info.relay_url to get the sub path

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants