Skip to content

Feat/a5 hooks channels - #822

Open
XanderD99 wants to merge 6 commits into
tidwall:masterfrom
GO-VIRTUAL-bv:feat/a5-hooks-channels
Open

Feat/a5 hooks channels#822
XanderD99 wants to merge 6 commits into
tidwall:masterfrom
GO-VIRTUAL-bv:feat/a5-hooks-channels

Conversation

@XanderD99

Copy link
Copy Markdown
Contributor

Please ensure you adhere to every item in this list

  • This PR was pre-approved by the project maintainer
  • I have self-reviewed the code
  • I have added all necessary tests

Describe your changes

Follow-up to the A5 index (#817): A5 is both an output keyword (A5 <resolution>, like HASHES) and a search area keyword (A5 <cell>, like HASH). The output parser always won, so the cell ID was consumed as a resolution and A5 <cell> never reached the area parser:

SETCHAN chan WITHIN fleet FENCE A5 51575d8000000000
> ERR invalid argument '51575d8000000000'

INTERSECTS key A5 <cell> failed the same way — only the workaround of naming an output first (IDS A5 <cell>) worked.

Fixed in parseSearchScanBaseTokens using the same lookahead BOUNDS already uses for its identical collision: on WITHIN/INTERSECTS, if the token after A5 isn't a valid resolution (0–30), it's a cell ID, so A5 is left to the area parser. SETHOOK/SETCHAN route through the same function, so hooks and channels are covered by the one change.

Also added the A5 case to parseArea, so TEST ... A5 <cell> works alongside HASH/QUADKEY/TILE. CLIP is rejected there, since a pentagon isn't a rectangle.

Tests: channel and webhook fences over an A5 cell (enter/inside/exit/outside, plus "a5" in the message when the output form is used), SETCHAN/SETHOOK parsing with and without an output, CHANS/HOOKS round-trip, error cases, an AOF-replay test since the stored fence command is re-parsed on restart, and TEST coverage matching the existing QUADKEY cases. All fail without the changes.

Issue number and link

#817#817

A5 was parsed only as an output keyword, so SETHOOK/SETCHAN ... FENCE A5 <cell> never reached the area parser.
A5 is now only ever a search area, so the resolution-vs-cell guess is gone.
Hex-parseable ids like "1" carry no resolution marker and reached a5-go with a negative resolution, crashing the server from any client.
@XanderD99

Copy link
Copy Markdown
Contributor Author

Good call — reworked to use the existing format convention instead of guessing.

A5S <zoom> is now the return format, alongside POINTS/IDS/HASHES/OBJECTS, and A5 <cell> is only ever a search area. The parser no longer has to decide whether the token after A5 is a resolution or a cell id, so WITHIN/INTERSECTS and the hooks and channels they back all read the same way:

INTERSECTS fleet A5S 10 A5 51575d8000000000
SETCHAN c INTERSECTS fleet FENCE A5S 10 A5 51575d8000000000

GET key id A5 <zoom> and SET key id A5 <cell> stay as they were — those mirror HASH and never collided.

Two things fell out while testing it:

  • The A5 output was never wired into the JSON envelope, so SCAN key A5 10 with OUTPUT json returned malformed JSON ({"ok":true{"id":"a",...}]) and silently dropped WITHFIELDS fields. It now emits "a5s":[...] like every other output.
  • A5 1 panicked the server. 1 parses as hex but carries no resolution marker, so GetResolution returns -1 and a5-go reaches SToAnchor with resolution -2 → makeslice: cap out of range. Reachable from any client through INTERSECTS, WITHIN, TEST or SET. a5DecodeCell now validates the resolution and origin bits, with a sweep test asserting nothing it accepts can panic the library.

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