Skip to content

fix: restore the applyRules call dropped in a refactor (ruleset regexRules and injections never run) - #155

Open
manantlerio wants to merge 1 commit into
everywall:mainfrom
manantlerio:fix/applyrules-never-called
Open

manantlerio wants to merge 1 commit into
everywall:mainfrom
manantlerio:fix/applyrules-never-called

Conversation

@manantlerio

Copy link
Copy Markdown

Summary

regexRules and injections in a ruleset have not run since 8fa0531
("#109 add base_url feature", 2026-04-12).

That commit refactored the URL rewriting in rewriteHtml onto the new
proxyPrefix variable, and the tail of the function went with it:

    if os.Getenv("RULESET") != "" {
        body = applyRules(body, rule)
    }
    return body

became

    return body

applyRules has had no callers since. Rulesets are still loaded, parsed and
validated, and /ruleset still serves them, but the regexRules and
injections in them are then dropped on the floor. Only the header, urlMods
and googleCache parts of a rule still do anything.

This makes paywall removal weaker than the README describes, and it fails in a
confusing way: a rule that looks correct simply has no effect, which reads like
a bad selector rather than a missing function call.

Why restore rather than delete

I went looking to remove dead code and found a regression instead. This is not
a feature that was retired, it is one line lost in a refactor, and git log -S
puts it in a single commit with a clear before and after. Deleting applyRules
would make the loss permanent and quietly narrow what a ruleset can express, so
restoring the call is the honest fix.

If you would rather this feature stayed off, say so and I will send the deletion
plus a README correction instead.

Two changes to how it comes back

The guard is now the loaded ruleset, not the environment variable.
applyRules already returns early when len(rulesSet) == 0, and the original
os.Getenv("RULESET") != "" check missed rulesets supplied with --ruleset,
which sets rulesSet via ProxySite without setting the variable. So the
original guard had a second, smaller bug in it.

applyRules can no longer take the process down. This is the part I would
most like a second opinion on, because it is slightly more than a revert, but
re-enabling the code path without it would turn a dormant function into a way to
kill the server:

  • regexp.MustCompile(regexRule.Match) panics on a malformed pattern. That
    pattern comes from a ruleset file, which RULESET may point at over HTTP, so
    it is untrusted input. Now regexp.Compile, logged and skipped.
  • log.Fatal(err) on a goquery parse or render error exits the process, on
    input that is just a page somebody asked to read. Now logged and skipped.

There is also a small correctness fix in the rendering path: the original did
body, err = doc.Html() and, on error, would have assigned the empty result
before checking. The new code only assigns on success.

Tests

Five new tests in handlers/applyrules_test.go: regexRules, injections,
an invalid pattern being skipped rather than panicking, the no-ruleset no-op,
and rewriteHtml actually applying rules end to end.

Two of them fail against current main, so they pin the regression rather than
just describing the fix:

--- FAIL: TestApplyRulesInvalidRegexIsSkipped   panic: regexp: Compile(`[unclosed`)
--- FAIL: TestRewriteHtmlAppliesRuleset         ruleset regexRules must reach rewriteHtml output

go vet, gofmt, go build ./... clean and go test ./... green in
golang:1.26.

Relationship to my other PRs

Independent of #154 (different files). Touches handlers/proxy.go like #153
does, but different regions of it, so they should not conflict. Happy to rebase
whichever you take second.

regexRules and injections in a ruleset have done nothing since 8fa0531
("everywall#109 add base_url feature", 2026-04-12). That commit refactored the URL
rewriting in rewriteHtml onto the new proxyPrefix variable and removed the
tail of the function along the way:

    if os.Getenv("RULESET") != "" {
        body = applyRules(body, rule)
    }
    return body

became

    return body

applyRules has had no callers since, so every rule's regexRules and injections
were parsed, validated and then ignored. That makes paywall removal noticeably
weaker than the README describes, and it is the kind of thing that looks like a
broken rule rather than a missing call.

This restores the call. Two changes to how it comes back:

The guard is now the loaded ruleset rather than os.Getenv("RULESET"). applyRules
already returns early when no ruleset is loaded, and the environment check
missed rulesets supplied with the --ruleset flag, which set rulesSet without
setting the variable.

applyRules can no longer take the process down. It used regexp.MustCompile on a
pattern that comes from a ruleset file, which may have been fetched from a
remote URL, so a malformed pattern panicked the request. It also called
log.Fatal on a goquery parse or render error, which exits the process, on input
that is just a page someone asked to read. Both now log and skip. Re-enabling
this code path without that would have turned a dormant function into a way to
kill the server with a bad rule or an unparseable page.

Tests cover regexRules, injections, an invalid pattern being skipped, the
no-ruleset no-op, and rewriteHtml actually applying rules. The last two fail
against current main.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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