Fix did:web encoding/decoding (#3454)

.changeset/gorgeous-lies-warn.md

@@ -0,0 +1,5 @@
+---
+"@atproto-labs/did-resolver": patch
+---
+
+Improve error descriptions

.changeset/lovely-news-prove.md

@@ -0,0 +1,5 @@
+---
+"@atproto/did": patch
+---
+
+Fix encoding and decoding of did:web

packages/did/jest.config.js

@@ -0,0 +1,13 @@
+/** @type {import('jest').Config} */
+module.exports = {
+  displayName: 'PDS',
+  transform: { '^.+\\.(t|j)s$': '@swc/jest' },
+  // Jest requires all ESM dependencies to be transpiled (even if they are
+  // dynamically import()ed).
+  transformIgnorePatterns: [
+    `/node_modules/.pnpm/(?!(get-port|lande|toygrad)@)`,
+  ],
+  testTimeout: 60000,
+  setupFiles: ['<rootDir>/../../jest.setup.ts'],
+  moduleNameMapper: { '^(\\.\\.?\\/.+)\\.js$': ['$1.ts', '$1.js'] },
+}

packages/did/package.json

@@ -28,9 +28,12 @@
     "zod": "^3.23.8"
   },
   "devDependencies": {
+    "@swc/jest": "^0.2.24",
+    "jest": "^28.1.2",
     "typescript": "^5.6.3"
   },
   "scripts": {
-    "build": "tsc --build tsconfig.build.json"
+    "build": "tsc --build tsconfig.build.json",
+    "test": "jest"
   }
 }

packages/did/src/methods/plc.ts

@@ -28,17 +28,17 @@ export function assertDidPlc(input: unknown): asserts input is Did<'plc'> {
     throw new InvalidDidError(typeof input, `DID must be a string`)
   }
 
+  if (!input.startsWith(DID_PLC_PREFIX)) {
+    throw new InvalidDidError(input, `Invalid did:plc prefix`)
+  }
+
   if (input.length !== DID_PLC_LENGTH) {
     throw new InvalidDidError(
       input,
       `did:plc must be ${DID_PLC_LENGTH} characters long`,
     )
   }
 
-  if (!input.startsWith(DID_PLC_PREFIX)) {
-    throw new InvalidDidError(input, `Invalid did:plc prefix`)
-  }
-
   // The following check is not necessary, as the check below is more strict:
 
   // assertDidMsid(input, DID_PLC_PREFIX.length)

packages/did/src/methods/web.ts

@@ -9,9 +9,11 @@ export const DID_WEB_PREFIX = `did:web:` satisfies Did<'web'>
 export function isDidWeb(input: unknown): input is Did<'web'> {
   // Optimization: make cheap checks first
   if (typeof input !== 'string') return false
+  if (!input.startsWith(DID_WEB_PREFIX)) return false
+  if (input.charAt(DID_WEB_PREFIX.length) === ':') return false
 
   try {
-    assertDidWeb(input)
+    didWebToUrl(input as Did<'web'>)
     return true
   } catch {
     return false
@@ -28,25 +30,31 @@ export function assertDidWeb(input: unknown): asserts input is Did<'web'> {
     throw new InvalidDidError(typeof input, `DID must be a string`)
   }
 
-  void didWebToUrl(input)
-}
-
-export function didWebToUrl(did: string): URL {
-  if (!did.startsWith(DID_WEB_PREFIX)) {
-    throw new InvalidDidError(did, `did:web must start with ${DID_WEB_PREFIX}`)
+  if (!input.startsWith(DID_WEB_PREFIX)) {
+    throw new InvalidDidError(input, `Invalid did:web prefix`)
   }
 
-  if (did.charAt(DID_WEB_PREFIX.length) === ':') {
-    throw new InvalidDidError(did, 'did:web MSID must not start with a colon')
+  if (input.charAt(DID_WEB_PREFIX.length) === ':') {
+    throw new InvalidDidError(input, 'did:web MSID must not start with a colon')
   }
 
+  void didWebToUrl(input as Did<'web'>)
+}
+
+export function didWebToUrl(did: Did<'web'>) {
   // Make sure every char is valid (per DID spec)
   assertDidMsid(did, DID_WEB_PREFIX.length)
 
+  const hostIdx = DID_WEB_PREFIX.length
+  const pathIdx = did.indexOf(':', hostIdx)
+
+  const host = pathIdx === -1 ? did.slice(hostIdx) : did.slice(hostIdx, pathIdx)
+  const path = pathIdx === -1 ? '' : did.slice(pathIdx)
+
   try {
-    const msid = did.slice(DID_WEB_PREFIX.length)
-    const parts = msid.split(':').map(decodeURIComponent)
-    const url = new URL(`https://${parts.join('/')}`)
+    const url = new URL(
+      `https://${host.replaceAll('%3A', ':')}${path.replaceAll(':', '/')}`,
+    ) as URL & { protocol: 'http:' | 'https:' }
     if (url.hostname === 'localhost') {
       url.protocol = 'http:'
     }
@@ -57,10 +65,8 @@ export function didWebToUrl(did: string): URL {
 }
 
 export function urlToDidWeb(url: URL): Did<'web'> {
-  const path =
-    url.pathname === '/'
-      ? ''
-      : url.pathname.slice(1).split('/').map(encodeURIComponent).join(':')
+  const port = url.port ? `%3A${url.port}` : ''
+  const path = url.pathname === '/' ? '' : url.pathname.replaceAll('/', ':')
 
-  return `did:web:${encodeURIComponent(url.host)}${path ? `:${path}` : ''}`
+  return `did:web:${url.hostname}${port}${path}`
 }

packages/did/tests/methods/plc.test.ts

@@ -0,0 +1,72 @@
+import { InvalidDidError } from '../../src/did-error.js'
+import { Did } from '../../src/did.js'
+import { asDidPlc, assertDidPlc, isDidPlc } from '../../src/methods/plc.js'
+
+const VALID: Did<'plc'>[] = [
+  'did:plc:l3rouwludahu3ui3bt66mfvj',
+  'did:plc:aaaaaaaaaaaaaaaaaaaaaaaa',
+  'did:plc:zzzzzzzzzzzzzzzzzzzzzzzz',
+]
+
+const INVALID: [value: unknown, message: string][] = [
+  ['did:plc:l3rouwludahu3ui3bt66mfv0', 'Invalid character at position 31'],
+  ['did:plc:l3rouwludahu3ui3bt66mfv1', 'Invalid character at position 31'],
+  ['did:plc:l3rouwludahu3ui3bt66mfv9', 'Invalid character at position 31'],
+  ['did:plc:l3rouwludahu3ui3bt66mfv', 'did:plc must be 32 characters long'],
+  ['did:plc:l3rouwludahu3ui3bt66mfvja', 'did:plc must be 32 characters long'],
+  ['did:plc:example.com:', 'did:plc must be 32 characters long'],
+  ['did:plc:exam%3Aple.com%3A8080', 'did:plc must be 32 characters long'],
+  [3, 'DID must be a string'],
+  [{ toString: () => 'did:plc:foo.com' }, 'DID must be a string'],
+  [[''], 'DID must be a string'],
+  ['random-string', 'Invalid did:plc prefix'],
+  ['did plc', 'Invalid did:plc prefix'],
+  ['lorem ipsum dolor sit', 'Invalid did:plc prefix'],
+]
+
+describe('isDidPlc', () => {
+  it('returns true for various valid dids', () => {
+    for (const did of VALID) {
+      expect(isDidPlc(did)).toBe(true)
+    }
+  })
+
+  it('returns false for invalid dids', () => {
+    for (const [did] of INVALID) {
+      expect(isDidPlc(did)).toBe(false)
+    }
+  })
+})
+
+describe('assertDidPlc', () => {
+  it('does not throw on valid dids', () => {
+    for (const did of VALID) {
+      expect(() => assertDidPlc(did)).not.toThrow()
+    }
+  })
+
+  it('throws if called with non string argument', () => {
+    for (const [val, message] of INVALID) {
+      expect(() => assertDidPlc(val)).toThrowError(
+        new InvalidDidError(
+          typeof val === 'string' ? val : typeof val,
+          message,
+        ),
+      )
+    }
+  })
+})
+
+describe('asDidPlc', () => {
+  it('returns the input for valid dids', () => {
+    for (const did of VALID) {
+      expect(asDidPlc(did)).toBe(did)
+    }
+  })
+
+  it('throws if called with invalid dids', () => {
+    for (const [val] of INVALID) {
+      expect(() => asDidPlc(val)).toThrowError(InvalidDidError)
+    }
+  })
+})

packages/did/tests/methods/web.test.ts

@@ -0,0 +1,109 @@
+import { InvalidDidError } from '../../src/did-error.js'
+import { Did } from '../../src/did.js'
+import {
+  asDidWeb,
+  assertDidWeb,
+  didWebToUrl,
+  isDidWeb,
+  urlToDidWeb,
+} from '../../src/methods/web.js'
+
+const VALID: [Did<'web'>, string][] = [
+  ['did:web:example.com', 'https://example.com/'],
+  ['did:web:sub.example.com', 'https://sub.example.com/'],
+  ['did:web:example.com%3A8080', 'https://example.com:8080/'],
+  [
+    'did:web:example.com:path:to:resource',
+    'https://example.com/path/to/resource',
+  ],
+  [
+    'did:web:example.com%3A8080:path:to:resource',
+    'https://example.com:8080/path/to/resource',
+  ],
+  [
+    'did:web:xn--b48h.com%3A8080:path:to:resource',
+    'https://🙃.com:8080/path/to/resource',
+  ],
+]
+
+const INVALID: [value: unknown, message: string][] = [
+  ['did:web:', 'DID method-specific id must not be empty'],
+  ['did:web:[email protected]', 'Disallowed character in DID at position 11'],
+  ['did:web::example.com', 'did:web MSID must not start with a colon'],
+  ['did:web:example.com:', 'DID cannot end with ":"'],
+  ['did:web:exam%3Aple.com%3A8080', 'Invalid Web DID'],
+  [3, 'DID must be a string'],
+  [{ toString: () => 'did:web:foo.com' }, 'DID must be a string'],
+  [[''], 'DID must be a string'],
+  ['random-string', 'Invalid did:web prefix'],
+  ['did web', 'Invalid did:web prefix'],
+  ['lorem ipsum dolor sit', 'Invalid did:web prefix'],
+]
+
+describe('isDidWeb', () => {
+  it('returns true for various valid dids', () => {
+    for (const [did] of VALID) {
+      expect(isDidWeb(did)).toBe(true)
+    }
+  })
+
+  it('returns false for invalid dids', () => {
+    for (const did of INVALID) {
+      expect(isDidWeb(did)).toBe(false)
+    }
+  })
+})
+
+describe('assertDidWeb', () => {
+  it('does not throw on valid dids', () => {
+    for (const [did] of VALID) {
+      expect(() => assertDidWeb(did)).not.toThrow()
+    }
+  })
+
+  it('throws if called with non string argument', () => {
+    for (const [val, message] of INVALID) {
+      expect(() => assertDidWeb(val)).toThrowError(
+        new InvalidDidError(
+          typeof val === 'string' ? val : typeof val,
+          message,
+        ),
+      )
+    }
+  })
+})
+
+describe('didWebToUrl', () => {
+  it('converts valid did:web to URL', () => {
+    for (const [did, url] of VALID) {
+      expect(didWebToUrl(did)).toStrictEqual(new URL(url))
+    }
+  })
+})
+
+describe('urlToDidWeb', () => {
+  it('converts URL to valid did:web', () => {
+    for (const [did, url] of VALID) {
+      expect(urlToDidWeb(new URL(url))).toBe(did)
+    }
+  })
+})
+
+describe('asDidWeb', () => {
+  it('returns the input for valid dids', () => {
+    for (const [did] of VALID) {
+      expect(asDidWeb(did)).toBe(did)
+    }
+  })
+
+  it('throws if called with invalid dids', () => {
+    for (const [val, message] of INVALID) {
+      expect(() => asDidWeb(val)).toThrowError(
+        new InvalidDidError(
+          typeof val === 'string' ? val : typeof val,
+          message,
+        ),
+      )
+    }
+  })
+})

packages/internal/did-resolver/src/did-resolver-base.ts

@@ -1,4 +1,4 @@
-import { FetchRequestError } from '@atproto-labs/fetch'
+import { FetchError, FetchResponseError } from '@atproto-labs/fetch'
 import { Did, DidError, extractDidMethod } from '@atproto/did'
 import { ZodError } from 'zod'
 
@@ -46,7 +46,12 @@ export class DidResolverBase<M extends string = string>
 
       return document as ResolvedDocument<D, M>
     } catch (err) {
-      if (err instanceof FetchRequestError) {
+      if (err instanceof FetchResponseError) {
+        const status = err.response.status >= 500 ? 502 : err.response.status
+        throw new DidError(did, err.message, 'did-fetch-error', status, err)
+      }
+
+      if (err instanceof FetchError) {
         throw new DidError(did, err.message, 'did-fetch-error', 400, err)
       }
 

packages/internal/did-resolver/src/methods/plc.ts

@@ -45,6 +45,7 @@ export class DidPlcMethod implements DidMethod<'plc'> {
     // should still check if the msid is valid.
     assertDidPlc(did)
 
+    // Should never throw
     const url = new URL(`/${encodeURIComponent(did)}`, this.plcDirectoryUrl)
 
     return this.fetch(url, {

packages/internal/did-resolver/src/methods/web.ts

@@ -6,7 +6,7 @@ import {
   fetchOkProcessor,
 } from '@atproto-labs/fetch'
 import { pipe } from '@atproto-labs/pipe'
-import { Did, didDocumentValidator, didWebToUrl } from '@atproto/did'
+import { Did, didDocumentValidator, DidError, didWebToUrl } from '@atproto/did'
 
 import { DidMethod, ResolveDidOptions } from '../did-method.js'
 
@@ -38,8 +38,10 @@ export class DidWebMethod implements DidMethod<'web'> {
     const didDocumentUrl = buildDidWebDocumentUrl(did)
 
     if (!this.allowHttp && didDocumentUrl.protocol === 'http:') {
-      throw new Error(
-        `Cannot resolve DID document for localhost: ${didDocumentUrl}`,
+      throw new DidError(
+        did,
+        'Resolution of "http" did:web is not allowed',
+        'did-web-http-not-allowed',
       )
     }