Skip to content
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

NDKNwc.fromURI always throws 'pubkey not set' in browser #285

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ekzyis
Copy link

@ekzyis ekzyis commented Nov 29, 2024

Description

The URL constructor in the browser parses URLs differently than in node.

With the current implementation of NWC in NDK, I don't see a way to get NWC working in the browser using NdkNWC.fromURI. I always get pubkey not set because NDK uses

pubkey: u.host ?? u.pathname

instead of

pubkey: u.host || u.pathname

here:

ndk/ndk/src/nwc/index.ts

Lines 94 to 110 in 6b3ea8b

static async fromURI(ndk: NDK, uri: string): Promise<NDKNwc> {
const u = new URL(uri);
// validate protocol nostr+walletconnect
if (u.protocol !== "nostr+walletconnect:") {
throw new Error("Invalid protocol");
}
const nwc = new NDKNwc({
ndk,
pubkey: u.host ?? u.pathname,
relayUrls: u.searchParams.getAll("relay") ?? [""],
secret: u.searchParams.get("secret") ?? "",
});
return nwc;
}

For example, if I use nostr+walletconnect://:

node: u.host ✅ | u.pathname

$ node
Welcome to Node.js v20.18.0.
Type ".help" for more information.
> new URL("nostr+walletconnect://2eb69717e2dac6c052ba7ab8cb164c5886c1ddaf3161b53e72d31cd8695a7715?relay=wss%3A%2F%2Frelay.primal.net&secret=2683ed09282e1b0d1545dd39957add77ce489ca37fd5fbb8f7b2e6b039272b88")
URL {
  href: 'nostr+walletconnect://2eb69717e2dac6c052ba7ab8cb164c5886c1ddaf3161b53e72d31cd8695a7715?relay=wss%3A%2F%2Frelay.primal.net&secret=2683ed09282e1b0d1545dd39957add77ce489ca37fd5fbb8f7b2e6b039272b88',
  origin: 'null',
  protocol: 'nostr+walletconnect:',
  username: '',
  password: '',
  host: '2eb69717e2dac6c052ba7ab8cb164c5886c1ddaf3161b53e72d31cd8695a7715',
  hostname: '2eb69717e2dac6c052ba7ab8cb164c5886c1ddaf3161b53e72d31cd8695a7715',
  port: '',
  pathname: '',
  search: '?relay=wss%3A%2F%2Frelay.primal.net&secret=2683ed09282e1b0d1545dd39957add77ce489ca37fd5fbb8f7b2e6b039272b88',
  searchParams: URLSearchParams {
    'relay' => 'wss://relay.primal.net',
    'secret' => '2683ed09282e1b0d1545dd39957add77ce489ca37fd5fbb8f7b2e6b039272b88' },
  hash: ''
}

browser: u.host ❌ | u.pathname ❌ (includes //)

2024-11-29-041912_1376x256_scrot

If I use nostr+walletconnect::

node: u.host ❌ | u.pathname

> new URL("nostr+walletconnect:2eb69717e2dac6c052ba7ab8cb164c5886c1ddaf3161b53e72d31cd8695a7715?relay=wss%3A%2F%2Frelay.primal.net&secret=2683ed09282e1b0d1545dd39957add77ce489ca37fd5fbb8f7b2e6b039272b88")
URL {
  href: 'nostr+walletconnect:2eb69717e2dac6c052ba7ab8cb164c5886c1ddaf3161b53e72d31cd8695a7715?relay=wss%3A%2F%2Frelay.primal.net&secret=2683ed09282e1b0d1545dd39957add77ce489ca37fd5fbb8f7b2e6b039272b88',
  origin: 'null',
  protocol: 'nostr+walletconnect:',
  username: '',
  password: '',
  host: '',
  hostname: '',
  port: '',
  pathname: '2eb69717e2dac6c052ba7ab8cb164c5886c1ddaf3161b53e72d31cd8695a7715',
  search: '?relay=wss%3A%2F%2Frelay.primal.net&secret=2683ed09282e1b0d1545dd39957add77ce489ca37fd5fbb8f7b2e6b039272b88',
  searchParams: URLSearchParams {
    'relay' => 'wss://relay.primal.net',
    'secret' => '2683ed09282e1b0d1545dd39957add77ce489ca37fd5fbb8f7b2e6b039272b88' },
  hash: ''
}

browser: u.host ❌ | u.pathname

2024-11-29-042030_1362x257_scrot

Since in both cases, the browser will set u.host to the empty string, the pubkey is never set in the browser. Using || instead of ?? fixes that at least for the nostr+walletconnect: case.

Additional context

Alby replaces the protocol with http:// to get consistent URL parsing behavior in node and browser for any NWC prefix, see here. I recommend to do the same but I didn't want to change this code more than necessary to get it working using at least one prefix before feedback.

Environment

NDK version: v2.10.5
node version: 20.18.0
Brave version: 1.62.165 Chromium: 121.0.6167.184 (Official Build) (64-bit)
Firefox version: 132.0.2 (64-bit)

@ekzyis ekzyis changed the title NdkNWC.fromURI always throws 'pubkey not set' in browser NDKNwc.fromURI always throws 'pubkey not set' in browser Nov 29, 2024
@ekzyis ekzyis mentioned this pull request Nov 29, 2024
4 tasks
@ekzyis
Copy link
Author

ekzyis commented Nov 29, 2024

FYI, there has been a breaking change to URL parsing in Chromium-based browsers in the Oct 15 release:

Support non-special scheme URLs

Previously, Chrome's URL parser didn't support non-special URLs. The parser parses non-special URLs as if they had an "opaque path", which is not aligned with the URL Standard. Now, Chromium's URL parser parses non-special URLs correctly, following the URL Standard.

-- https://developer.chrome.com/release-notes/130?hl=en#support_non-special_scheme_urls

Apparently, they didn't follow the URL standard but do now. However, this doesn't seem to be limited to Chromium-based browsers since the Developer Edition of Firefox also included breaking changes, see stackernews/stacker.news#1590 (comment).

Thanks to @riccardobl for investigating this discrepancy between URL parsing between browsers and even between recent versions of the same browser.

See https://docs.google.com/document/u/0/d/1LjxHl32fE4tCKugrK_PIso7mfXQVEeoD1wSnX2y0ZU8/mobilebasic?resourcekey=0-d1gP4X2sG7GPl9mlTeptIA#heading=h.a67ulu2yrl9p for more details.

@ekzyis ekzyis force-pushed the fix-nwc-url-parsing branch from 0ae5bb2 to 0c36b35 Compare December 4, 2024 00:27
@ekzyis
Copy link
Author

ekzyis commented Dec 9, 2024

Same issue exists with the signer for NIP-46

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.

1 participant