nandi/frq · 32dfa6e

nandi/frqpublic⑂ Fork 0

⑂ 32dfa6e

A handle is scanned, not matched

`std/re` is PCRE, and PCRE is a shared library the machine running the
tests may not have. It did not: the profile commit went green here and
red in CI, on `could not load: libpcre.so(.3|.1|)` — a runtime failure,
so nothing caught it at compile time.

`isHandle` now scans the domain shape directly, which is what
`ircparse` and `clock` already do and for the same reason. The grammar
it accepts is unchanged; the tests say so more thoroughly than before,
since hyphens and empty labels were the regex's business and are now
ours.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

nandi committed 2026-09-19T08:17:23-07:00 Browse files

32dfa6e parent: 9c1dfe9

modified nim/src/frq/profile.nim +33 -5 Viewed

@@ -18,7 +18,7 @@
18	18	## The Clojure has a fetch seam here because the two compilers disagreed about
19	19	## HTTP. Nim has one client, so the seam is gone and `fetch` simply asks.
20	20
21		-import std/[json, re, strutils, tables]
	21	+import std/[json, strutils, tables]
22	22	from std/unicode import runeLen, runeSubStr
23	23	import frq/[atproto, trace]
24	24
@@ -33,11 +33,39 @@ type
33	33
34	34	var cache: Table[string, Profile]
35	35
36		-let handlePattern = re"^[A-Za-z0-9][A-Za-z0-9-](\.[A-Za-z0-9][A-Za-z0-9-])*\.[A-Za-z]{2,}$"
37		-
38		-proc isHandle*(nick: string): bool =
	36	+func isHandle*(nick: string): bool =
39	37	## Whether this nick is an AT Protocol handle, and so worth a lookup.
40		- nick.len > 0 and nick.match(handlePattern)
	38	+ ##
	39	+ ## Domain shape, per the handle grammar: dot-separated labels that begin
	40	+ ## with a letter or digit and may then carry hyphens, and a final label of
	41	+ ## letters only. Scanned rather than matched: `std/re` is PCRE, which means
	42	+ ## a `libpcre.so` the machine running this may not have — and it did not,
	43	+ ## the first time this shipped. `ircparse` and `clock` scan for the same
	44	+ ## reason.
	45	+ if nick.len == 0: return false
	46	+ var
	47	+ labels = 0
	48	+ labelLen = 0
	49	+ lastAllAlpha = true
	50	+ for i, c in nick:
	51	+ if c == '.':
	52	+ if labelLen == 0: return false # empty label: leading, doubled or trailing dot
	53	+ labels.inc
	54	+ labelLen = 0
	55	+ lastAllAlpha = true
	56	+ elif c in {'A'..'Z', 'a'..'z'}:
	57	+ labelLen.inc
	58	+ elif c in {'0'..'9'}:
	59	+ labelLen.inc
	60	+ lastAllAlpha = false
	61	+ elif c == '-':
	62	+ if labelLen == 0: return false # a label may not open with a hyphen
	63	+ labelLen.inc
	64	+ lastAllAlpha = false
	65	+ else:
	66	+ return false
	67	+ # The last label is the TLD: at least two characters, and all letters.
	68	+ labels > 0 and labelLen >= 2 and lastAllAlpha
41	69
42	70	proc actorFor*(did, nick: string): string =
43	71	## The identity to look a profile up by, or "" when there is none.

  ## The Clojure has a fetch seam here because the two compilers disagreed about
  ## HTTP. Nim has one client, so the seam is gone and `fetch` simply asks.
--import std/[json, re, strutils, tables]
++import std/[json, strutils, tables]
  from std/unicode import runeLen, runeSubStr
  import frq/[atproto, trace]
  var cache: Table[string, Profile]
--let handlePattern = re"^[A-Za-z0-9][A-Za-z0-9-]*(\.[A-Za-z0-9][A-Za-z0-9-]*)*\.[A-Za-z]{2,}$"
++func isHandle*(nick: string): bool =
--
--proc isHandle*(nick: string): bool =
    ## Whether this nick is an AT Protocol handle, and so worth a lookup.
--  nick.len > 0 and nick.match(handlePattern)
++  ##
++  ## Domain shape, per the handle grammar: dot-separated labels that begin
++  ## with a letter or digit and may then carry hyphens, and a final label of
++  ## letters only. Scanned rather than matched: `std/re` is PCRE, which means
++  ## a `libpcre.so` the machine running this may not have — and it did not,
++  ## the first time this shipped. `ircparse` and `clock` scan for the same
++  ## reason.
++  if nick.len == 0: return false
++  var
++    labels = 0
++    labelLen = 0
++    lastAllAlpha = true
++  for i, c in nick:
++    if c == '.':
++      if labelLen == 0: return false     # empty label: leading, doubled or trailing dot
++      labels.inc
++      labelLen = 0
++      lastAllAlpha = true
++    elif c in {'A'..'Z', 'a'..'z'}:
++      labelLen.inc
++    elif c in {'0'..'9'}:
++      labelLen.inc
++      lastAllAlpha = false
++    elif c == '-':
++      if labelLen == 0: return false     # a label may not open with a hyphen
++      labelLen.inc
++      lastAllAlpha = false
++    else:
++      return false
++  # The last label is the TLD: at least two characters, and all letters.
++  labels > 0 and labelLen >= 2 and lastAllAlpha
  proc actorFor*(did, nick: string): string =
    ## The identity to look a profile up by, or "" when there is none.

modified nim/tests/tprofile.nim +14 -0 Viewed

@@ -17,6 +17,20 @@ suite "isHandle":
17	17	test "nor is something with no TLD":
18	18	check not isHandle("alice.")
19	19	check not isHandle("")
	20	+ test "a hyphen is fine inside a label, not at its start":
	21	+ check isHandle("my-host.example.com")
	22	+ check not isHandle("-host.example.com")
	23	+ check not isHandle("host.-example.com")
	24	+ test "the TLD is two or more letters, and only letters":
	25	+ check not isHandle("alice.c")
	26	+ check not isHandle("alice.c0m")
	27	+ check not isHandle("alice.com-")
	28	+ test "no empty labels":
	29	+ check not isHandle(".alice.com")
	30	+ check not isHandle("alice..com")
	31	+ test "nor anything a domain cannot hold":
	32	+ check not isHandle("alice bob.com")
	33	+ check not isHandle("alice@bsky.social")
20	34
21	35	suite "actorFor":
22	36	test "a DID wins, because it is the identity itself":

    test "nor is something with no TLD":
      check not isHandle("alice.")
      check not isHandle("")
++  test "a hyphen is fine inside a label, not at its start":
++    check isHandle("my-host.example.com")
++    check not isHandle("-host.example.com")
++    check not isHandle("host.-example.com")
++  test "the TLD is two or more letters, and only letters":
++    check not isHandle("alice.c")
++    check not isHandle("alice.c0m")
++    check not isHandle("alice.com-")
++  test "no empty labels":
++    check not isHandle(".alice.com")
++    check not isHandle("alice..com")
++  test "nor anything a domain cannot hold":
++    check not isHandle("alice bob.com")
++    check not isHandle("alice@bsky.social")
  suite "actorFor":
    test "a DID wins, because it is the identity itself":