Skip to content

pirania: fix dead captive portal redirect server (uhttpd-mod-lua contract) - #1288

Merged
a-gave merged 2 commits into
libremesh:masterfrom
luandro:fix/pirania-uhttpd-handle-request
Oct 2, 2026
Merged

a-gave merged 2 commits into
libremesh:masterfrom
luandro:fix/pirania-uhttpd-handle-request

Conversation

@luandro

@luandro luandro commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Problem

Pirania's HTTP capture redirect server (pirania-uhttpd, port 59080) never listens: procd gives up after five respawn attempts and every HTTP capture redirect lands on a closed port, so the captive portal is dead on any build that includes ba4f3824 ("fix: luacheck: pirania", merged 2026-08-05).

Root cause

That commit localized handle_request in packages/pirania/files/www/paradise... packages/pirania/files/www/pirania-redirect/redirect and added return handle_request.
That is valid Lua and it keeps the busted tests green, because they load the file as a module via require(). But the real consumer, uhttpd-mod-lua, uses a different contract: it loadfile()s the handler, executes it, and then looks up the global handle_request. With a local function the lookup fails, uhttpd exits with Error: Lua handler ... provides no handle_request() callback, procd respawns five times and gives up.

Fix

  • Keep handle_request global (with a -- luacheck: globals handle_request directive so lint stays clean) and keep the module-style return, so both consumers work.
  • Add a test that loads the file the way uhttpd does (loadfile() + assert global), so this regression cannot pass CI again.

Verification

Found live on LibreMesh master builds flashed onto UniFi 6 Lite nodes: /etc/init.d/pirania-uhttpd status = not running; manual invocation of the init command line reproduced Error: Lua handler /www/pirania-redirect/redirect provides no handle_request() callback instantly; after the fix the service stays up, port 59080 answers 302 to the portal URL, and /portal/auth.html answers 200. CI will also run the new contract test.

Also in this PR (second commit)

The packaged default catch_bridged_interfaces only listed wlan0-ap, so on two-radio nodes (UniFi 6 Lite) 5 GHz clients bypassed the portal. Added wlan1-ap; consumer is an nft set of type ifname, so the extra default entry is harmless on single-radio devices.

Commit ba4f382 ('fix: luacheck: pirania') localized handle_request and
added 'return handle_request'. That is valid Lua and it keeps the busted
tests passing, because they load the file as a module via require().
uhttpd-mod-lua, the actual consumer, uses a different contract: it
loadfile()s the handler, executes it and then looks up the GLOBAL
handle_request. With a local function the lookup fails, uhttpd exits
with 'provides no handle_request() callback', procd gives up after five
respawns and pirania-uhttpd never listens on :59080. Every HTTP capture
redirect then lands on a closed port and the captive portal is dead.

Keep the function global (with a luacheck directive so lint stays clean)
and keep the module-style return, so both consumers work. Add a test
that loads the file the way uhttpd does and asserts the global is
defined, so the regression cannot pass CI again.
The packaged default only listed wlan0-ap, so on two-radio LibreMesh
nodes (e.g. Ubiquiti UniFi 6 Lite) clients on the 5 GHz AP were never
captured by the captive portal. The consumer is an nft set of type
ifname, so a name that does not exist on single-radio devices never
matches and the extra entry is harmless there.

@a-gave a-gave left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Assuming it has been tested/fixed also on a real device: approving
And personally sorry for having introduced the regression, the luacheck globals is more appropriate!

@a-gave
a-gave merged commit cfcbcf4 into libremesh:master Oct 2, 2026
25 of 26 checks passed

This branch is waiting to be deployed

1 waiting deployment
physical-lab — a5d0ca99 Waiting Oct 2, 2026 by luandro via restore-lab-vlans #169
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.

2 participants