luci-app-xfrpc: add comment explaining GridSection type behavior#8704
Open
ximalu wants to merge 2 commits into
Open
luci-app-xfrpc: add comment explaining GridSection type behavior#8704ximalu wants to merge 2 commits into
ximalu wants to merge 2 commits into
Conversation
This is a documentation-only change. The GridSection type 'xfrpc' is intentional — the init script (xfrpc.init) is now fixed to handle xfrpc-typed proxy sections via a dedicated fallback handler. The actual functional fix is in openwrt/packages (xfrpc.init update). See issue openwrt#7972 for details.
ximalu
added a commit
to ximalu/packages
that referenced
this pull request
Jun 14, 2026
LuCI creates proxy sections with UCI type "xfrpc" (the GridSection type), but the init script only iterated type-specific sections (tcp, http, https, socks5). This caused proxy configurations added via the LuCI web interface to be silently dropped during config generation, producing a broken xfrpc.ini. Two fixes: 1. handle_xfrpc(): guard with [ "$section" = "common" ] || return to prevent leaking server_addr/token into non-common sections. 2. Added handle_xfrpc_proxy(): a fallback for xfrpc-typed sections that reads the "type" option and writes the correct proxy config. Fixes openwrt/luci#7972 Companion PR: openwrt/luci#8704
Fix 1: Remove unnecessary SectionValue+TypedSection wrapper that caused empty display (with "xfrp") or duplicate display (with "xfrpc"). Bind disabled and loglevel options directly to the common section. Fix 2: Rename field from "disabled" to "enabled" with label "Enable xfrpc service". The init script reads "enabled" (default 1), not "disabled", so the checkbox previously had no effect on service startup. Fixes openwrt#7972
openwrt-ai
reviewed
Jun 14, 2026
openwrt-ai
left a comment
openwrt-ai
left a comment
Collaborator
There was a problem hiding this comment.
Commit checks
3816f22"luci-app-xfrpc: fix startup settings section and field naming" — The body states "The init script reads 'enabled' (default 1)", but the backend default is0, not1:start_service()validates'enabled:bool:0'and the shippedxfrpc.confshipsoption enabled 0. The renamedisabled→enabledis correct (the daemon does readenabled), but the stated default is wrong and the LuCIo.default = '1'consequently diverges from the daemon default. See the inline note.
Generated by Claude Code
| o = s.taboption('init', form.Flag, 'enabled', _('Enable xfrpc service')); | ||
| o.datatype = 'bool'; | ||
| o.optional = true; | ||
| o.default = '1'; |
Collaborator
There was a problem hiding this comment.
The frontend defaults enabled to 1, but the backend defaults it to 0. start_service() validates 'enabled:bool:0' and the shipped xfrpc.conf sets option enabled 0. With rmempty = false the value is always written on save, so post-save behavior is fine, but the pre-save UI default disagrees with the daemon default (service off). Note the commit message claims "the init script reads 'enabled' (default 1)" — the init script default is actually 0. Is defaulting the LuCI checkbox to on intended?
Generated by Claude Code
| s.dynamic = true; | ||
|
|
||
| o = s.option(form.Flag, 'disabled', _('Disabled xfrpc service')); | ||
| o = s.taboption('init', form.Flag, 'enabled', _('Enable xfrpc service')); |
Collaborator
There was a problem hiding this comment.
nit: The PR title ("add comment explaining GridSection type behavior") and body ("This is a documentation-only change") describe only the first commit (efd9ee5). This commit (3816f22) is a functional change: it removes the SectionValue+TypedSection wrapper, rebinds loglevel onto the common section, and renames the disabled flag to enabled. Please update the PR description so it reflects the section restructuring and the option rename, not just the documentation comment.
Generated by Claude Code
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Failed checksIssues marked with an ❌ are failing checks. Commit 3816f22
Commit efd9ee5
For more details, see the full job log. Something broken? Consider providing feedback. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This is a documentation-only change to clarify that the GridSection type
'xfrpc'is intentional.Background
Issue #7972 reported that
luci-app-xfrpcgenerates configs that prevent xfrpc from starting.Investigation showed the root cause is a section type mismatch:
xfrpc(via GridSection)xfrpc.init) iterates sections by type (tcp,http,https,socks5)xfrpc, proxy configs were lost in the generatedxfrpc.iniFix Approach
The functional fix is in
openwrt/packages(net/xfrpc/files/xfrpc.init), which now has a fallback handler that reads thetypeoption fromxfrpc-typed sections and dispatches correctly. This LuCI-side change adds a comment documenting the contract between LuCI and the init script.Testing
Verified on ImmortalWrt 24.10.4 (Xiaomi WR30U). TCP proxy sections created via LuCI now generate correct
xfrpc.inifiles and the xfrpc service starts successfully.