gargoyle-firewall-util: fix weekly-range restriction rules never loading - #155
Merged
Conversation
…ables Two independent bugs in how active_weekly_ranges gets turned into an nft command, both dating to the original nftables rewrite -- affects every restriction/whitelist rule using the weekly schedule type in the general Restrictions form, not just the Family Time wizard. 1. The generated command used the keyword weekly-ranges (hyphenated). The nft timerange extensions own grammar (both its command-line parser and its own display output) uses weeklyranges, one word -- verified against netfilter-match-modules/nftables/timerange/libnftnl/ src_expr_timerange.c. Every weekly-range rule failed to load with syntax error, options must be specified before commands, silently: the rule stays enabled in UCI, just never actually enforces anything. 2. Independently, even with the keyword fixed, a standalone space- surrounded - token in the value (e.g. Day HH:MM - Day HH:MM, the canonical display format weekly_i18n() in restrictions.js needs to correctly localize day names) gets misread by nfts own CLI as an attempted option flag, hitting the identical error. Confirmed live against nft 1.1.6 by isolating each half: a well-formed value still failed until the spacing around the hyphen was collapsed. Fixed by sanitizing a copy of the value (collapsing - to -, reusing the dynamic_replace loop idiom already established in restore_quotas.c for the same class of problem) before it reaches the nft command line, and correcting the keyword -- the display format itself is unchanged, so weekly_i18n() keeps working. Verified end to end against the real compiled binary on a live router: a correctly-scheduled weekly-range rule now loads and is confirmed present in the live ruleset with the right decoded time values.
2 tasks
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
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.
Summary
make_nftables_rules.cmeant every restriction/whitelist rule using the "weekly" schedule type (the general Restrictions form's own long-existing option, not just the Family Time wizard) silently failed to load into nftables — enabled in UCI, no error surfaced anywhere, just never enforced.timerange weekly-ranges→ the nfttimerangeextension's own grammar (command-line parser and its own display output) isweeklyranges, one word.-token in the value (the canonicalDay HH:MM - Day HH:MMdisplay formatweekly_i18n()needs to correctly localize day names) gets misread by nft's own CLI as an attempted option flag.-to-, reusing thedynamic_replaceloop idiom already established inrestore_quotas.cfor the same class of problem) before it reaches the nft command line, and correcting the keyword — the display format itself is unchanged.Test plan
make package/gargoyle-firewall-util/{clean,prepare,compile}— clean, no new warnings