Skip to content

luci-app-https-dns-proxy: fix get_providers() invalid JSON in directory fallback#8648

Open
unglazed7010 wants to merge 1 commit into
openwrt:masterfrom
unglazed7010:fix-get-providers-trailing-comma
Open

luci-app-https-dns-proxy: fix get_providers() invalid JSON in directory fallback#8648
unglazed7010 wants to merge 1 commit into
openwrt:masterfrom
unglazed7010:fix-get-providers-trailing-comma

Conversation

@unglazed7010

Copy link
Copy Markdown

Fixes #8647

Summary

  • get_providers() emitted a trailing comma after every provider file in the directory-scan fallback, producing JSON rejected by strict parsers and silently breaking the provider list in the UI
  • An empty $providersDir caused cat to fail on a literal glob string while still emitting ,, producing [,]

Patch

root/usr/libexec/rpcd/luci.https-dns-proxy

  • Flip comma placement: emit separator before each item after the first (not after every item)
  • Add [ -f "$f" ] || continue guard and narrow glob to *.json so an empty/absent directory yields a valid empty array instead of broken output
  • Allow _LUCI_PROVIDERS_DIR / _LUCI_PROVIDERS_JSON env-var overrides (default unchanged) to make the fallback path exercisable in the test harness without touching live system paths

tests/run_tests.sh

New section 04 — three tests:

  1. Directory fallback output is valid JSON (python3 json parser accepts it)
  2. All 45 providers are present in the output (count check)
  3. Empty providers directory yields a valid empty array

Test run results

## 04: getProviders JSON output

  PASS: getProviders dir-fallback returns valid JSON
  PASS: getProviders dir-fallback: all 45 providers present
  PASS: getProviders empty dir-fallback: valid empty array

Ran 335 tests, 335 passed, 0 failed

(was 332 tests before this patch; all pre-existing tests still pass)

Signed-off-by: unglazed7010 thatblueghoul@gmail.com

…ry fallback

The directory-scan fallback in get_providers() emitted a trailing comma
after every provider file including the last, producing output like:

  {"https-dns-proxy":[{...},{...},]}

which is rejected by strict JSON parsers.  This code path is taken when
providers.json has not been pre-built, so the LuCI provider list would be
silently empty.

Fix the loop to emit the separator before each entry after the first,
and add [ -f ] guard + *.json glob so an empty or absent directory yields
a valid empty array instead of a broken [ , ] fragment.

Make providersDir/providersJson overridable via _LUCI_PROVIDERS_DIR /
_LUCI_PROVIDERS_JSON environment variables (no-op on production; used
only by the test harness) so the fallback path can be exercised without
touching live system paths.

Add three new tests in section 04 of tests/run_tests.sh:
- directory fallback produces valid JSON across all 45 real providers
- all providers are present in the output (count check)
- empty providers directory yields a valid empty array

Signed-off-by: unglazed7010 <thatblueghoul@gmail.com>
@github-actions

Copy link
Copy Markdown

Warning

Some formality checks failed.

Consider (re)reading submissions guidelines.

Failed checks

Issues marked with an ❌ are failing checks.

Commit 727cbdf

  • 🔶 Author name must be either a real name 'firstname lastname' or a nickname/alias/handle
    Actual: unglazed7010 seems to be a nickname or an alias
    Expected: a real name 'firstname lastname'
  • 🔶 Commit(ter) name must be either a real name 'firstname lastname' or a nickname/alias/handle
    Actual: unglazed7010 seems to be a nickname or an alias
    Expected: a real name 'firstname lastname'
  • 🔶 Commit subject must be <= 80 (and should be <= 60) characters long
    Actual: subject is 80 characters long
    $\textsf{luci-app-https-dns-proxy: fix get_providers() invalid JSON i\color{yellow}{n directory fallback}\color{red}{}}$

For more details, see the full job log.

Something broken? Consider providing feedback.

@egc112

egc112 commented May 22, 2026

Copy link
Copy Markdown
Contributor

Please direct your PR upstream to:
https://github.com/mossdef-org/luci-app-https-dns-proxy

Thanks for your contribution

@openwrt-ai openwrt-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed 1 new commit. This PR modifies luci-app-https-dns-proxy, which is maintained out-of-tree at https://github.com/mossdef-org/luci-app-https-dns-proxy and should be submitted there rather than reviewed/merged in openwrt/luci. See the inline comment for details; the underlying fix appears sound but belongs upstream.


Generated by Claude Code

[ -f "$f" ] || continue
[ "$first" = "1" ] || printf ','
first=0
cat "$f"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

luci-app-https-dns-proxy is maintained out-of-tree, not in openwrt/luci. Changes to this RPC backend (and the accompanying tests) should be submitted to the upstream repository so they don't get overwritten on the next sync: https://github.com/mossdef-org/luci-app-https-dns-proxy

The fix itself looks reasonable (the trailing-comma / [,] bug is real), but it needs to land upstream rather than here. Please redirect this PR there.


Generated by Claude Code

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.

luci-app-https-dns-proxy: get_providers() emits invalid JSON (trailing comma) in directory fallback

3 participants