Consider adding StatusNoContent and StatusNotModified guards to `filter.go`

[coderabbitai]

Context

This issue is a follow-up from PR ZenPrivacy/zen-desktop#434 and the discussion in ZenPrivacy/zen-desktop#434 (comment).

Problem

The filtering system in internal/filter/filter.go currently processes all HTTP responses for content injection (scriptlets, cosmetic rules, CSS rules, JS rules) without checking for specific status codes that shouldn't have their bodies modified.

Since there are multiple injectors (scriptletsInjector, cosmeticRulesInjector, cssRulesInjector, jsRuleInjector), we want to guard at a single centralized point rather than adding guards to each individual injector.

Proposed Enhancement

Consider adding guards in the HandleResponse method in filter.go to skip all injections for:

• 204 No Content: These responses explicitly have no body content
• 304 Not Modified: These responses typically don't include a body and shouldn't be modified

Current State

• Content-Type checking is already handled in the isDocumentNavigation function (text/html requirement)
• All injections happen within the if isDocumentNavigation(req, res) block in HandleResponse (lines ~283-314)
• The injector currently proceeds for any response with matching rules and proper content type
• Status code validation would add an additional safety layer at the centralized filtering point

Implementation Location

The guard should be added in internal/filter/filter.go in the HandleResponse method, either:

1. As an additional condition in the isDocumentNavigation function, or
2. As a separate check before the injections begin

Considerations

This enhancement requires careful consideration of:

1. Whether these status codes can realistically occur in the context where the injectors run
2. The performance impact of additional checks
3. Whether the existing filter logic already prevents these cases
4. Edge cases where these status codes might legitimately need processing

References

• Original discussion: Extended/procedural CSS zen-desktop#434 (comment)
• Requested by: @anfragment

[anfragment]

@coderabbitai since there are multiple "Injectors", we want to guard at a single point, in filter.go. Correct the issue accordingly.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

@anfragment I'll examine the codebase to understand the filtering architecture and then update the issue to reflect that the status code guards should be added in filter.go as the centralized point.

---

🏁 Script executed:

#!/bin/bash
# First, let's examine the filter.go file to understand the current filtering logic
echo "=== Examining filter.go ==="
fd filter.go --exec cat {}

echo -e "\n=== Looking for injector files to understand multiple injection points ==="
fd -e go injector --exec echo "File: {}" \; --exec head -20 {}

echo -e "\n=== Searching for references to status codes in the codebase ==="
rg -n "Status.*Content|Status.*Modified|204|304" --type=go

Length of output: 14275

---

🏁 Script executed:

#!/bin/bash
# Update the GitHub issue with the corrected approach
gh issue edit 444 --body "# Consider adding StatusNoContent and StatusNotModified guards to filter.go

## Context

This issue is a follow-up from PR ZenPrivacy/zen-desktop#434 and the discussion in https://github.com/ZenPrivacy/zen-desktop/pull/434#discussion_r2330373830.

## Problem

The filtering system in \`internal/filter/filter.go\` currently processes all HTTP responses for content injection (scriptlets, cosmetic rules, CSS rules, JS rules) without checking for specific status codes that shouldn't have their bodies modified.

Since there are multiple injectors (scriptletsInjector, cosmeticRulesInjector, cssRulesInjector, jsRuleInjector), we want to guard at a single centralized point rather than adding guards to each individual injector.

## Proposed Enhancement

Consider adding guards in the \`HandleResponse\` method in \`filter.go\` to skip all injections for:
- **204 No Content**: These responses explicitly have no body content  
- **304 Not Modified**: These responses typically don't include a body and shouldn't be modified

## Current State

- Content-Type checking is already handled in the \`isDocumentNavigation\` function (text/html requirement)
- All injections happen within the \`if isDocumentNavigation(req, res)\` block in \`HandleResponse\` (lines ~283-314)
- The injector currently proceeds for any response with matching rules and proper content type
- Status code validation would add an additional safety layer at the centralized filtering point

## Implementation Location

The guard should be added in \`internal/filter/filter.go\` in the \`HandleResponse\` method, either:
1. As an additional condition in the \`isDocumentNavigation\` function, or
2. As a separate check before the injections begin

## Considerations

This enhancement requires careful consideration of:
1. Whether these status codes can realistically occur in the context where the injectors run
2. The performance impact of additional checks  
3. Whether the existing filter logic already prevents these cases
4. Edge cases where these status codes might legitimately need processing

## References

- Original discussion: https://github.com/ZenPrivacy/zen-desktop/pull/434#discussion_r2330373830
- Requested by: @anfragment"

Length of output: 52

---

@anfragment I've corrected the issue description to reflect the proper architecture. The issue now correctly identifies that the status code guards should be added in filter.go at the centralized filtering point in the HandleResponse method, rather than in individual injectors.

The key changes in the updated issue:

1. Centralized approach: Guards should be in internal/filter/filter.go in the HandleResponse method since there are multiple injectors (scriptlets, cosmetic rules, CSS rules, JS rules)

2. Implementation location: Two potential approaches:

   • Add the status code check as an additional condition in the isDocumentNavigation function
   • Add a separate check before the injections begin (lines ~283-314)

3. Rationale: Since all injections currently happen within the if isDocumentNavigation(req, res) block, this is the logical place to add status code validation for 204 (No Content) and 304 (Not Modified) responses

This approach ensures we guard against unnecessary body modifications at a single point rather than duplicating the logic across multiple injectors, which is much cleaner and more maintainable.

---

✏️ Learnings added

Learnt from: anfragment
URL: https://github.com/ZenPrivacy/zen-core/issues/22

Timestamp: 2025-09-08T15:48:08.308Z
Learning: In the zen-desktop project, content injection guards should be implemented at the centralized filtering point in internal/filter/filter.go HandleResponse method, rather than in individual injectors, since there are multiple injectors (scriptlets, cosmetic rules, CSS rules, JS rules) that all process responses.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.