Skip to content

Fix domain validation checking wrong variable in functions domain loop#11676

Open
zaidxshaikh wants to merge 1 commit intoappwrite:1.9.xfrom
zaidxshaikh:fix-11675-proxy-domain-validation-typo
Open

Fix domain validation checking wrong variable in functions domain loop#11676
zaidxshaikh wants to merge 1 commit intoappwrite:1.9.xfrom
zaidxshaikh:fix-11675-proxy-domain-validation-typo

Conversation

@zaidxshaikh
Copy link
Copy Markdown

Summary

  • Fixed a typo bug in src/Appwrite/Platform/Modules/Proxy/Action.php where the empty() check inside the foreach loop was checking the outer variable $functionsDomains instead of the loop variable $functionsDomain
  • The sites domain loop (line 40) correctly checks $sitesDomain, but the functions domain loop (line 53) was incorrectly checking $functionsDomains

Impact

When _APP_DOMAIN_FUNCTIONS env var is set, empty entries from trailing/double commas are never skipped and get processed as invalid domain restrictions.

Fixes #11675

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 30, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 36d708b6-e95a-4bb7-ba92-3477484325f3

📥 Commits

Reviewing files that changed from the base of the PR and between 90b2563 and 3bb2c34.

📒 Files selected for processing (1)
  • src/Appwrite/Platform/Modules/Proxy/Action.php

📝 Walkthrough

Walkthrough

The change fixes a variable reference bug in the validateDomainRestrictions() method in src/Appwrite/Platform/Modules/Proxy/Action.php. The foreach loop over values from the _APP_DOMAIN_FUNCTIONS environment variable previously checked the outer variable $functionsDomains in its empty() condition instead of the loop variable $functionsDomain; the condition now correctly checks $functionsDomain so empty entries are skipped as intended, matching the pattern used for sites domains.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and accurately describes the main change: fixing a variable name bug in the domain validation loop.
Description check ✅ Passed The description is well-related to the changeset, providing context about the bug, its impact, and the specific lines affected.
Linked Issues check ✅ Passed The code changes directly address all objectives from issue #11675: changing empty($functionsDomains) to empty($functionsDomain) in the foreach loop.
Out of Scope Changes check ✅ Passed The changes are precisely scoped to fixing the identified bug in the domain validation loop with no extraneous modifications.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 30, 2026

Greptile Summary

This PR fixes a one-character variable name typo in validateDomainRestrictions() inside src/Appwrite/Platform/Modules/Proxy/Action.php. The foreach loop over the comma-split $functionsDomains string was guarding against empty entries with empty($functionsDomains) (the outer container) instead of empty($functionsDomain) (the loop variable). Because $functionsDomains is never empty when the env var is set, every empty token produced by trailing or double commas was incorrectly passed downstream as an invalid domain restriction. The fix aligns the functions loop with the already-correct sites loop on line 40.

  • Bug fixed: empty($functionsDomains)empty($functionsDomain) on line 53, mirroring the pattern already used for $sitesDomain on line 40.
  • Impact: Without the fix, empty tokens from malformed _APP_DOMAIN_FUNCTIONS values (e.g. domain.com,) bypassed the skip guard and were fed into ValidatorDomain::createRestriction() as empty strings, potentially causing incorrect validation behaviour.
  • Scope: Change is a single-token correction with no side effects on surrounding logic.

Confidence Score: 5/5

Safe to merge — minimal, correct one-token fix with no logic regressions.

The change is a single variable-name correction that directly parallels the already-correct pattern used for the sites domain loop. No new logic is introduced, and no other issues were found in the file.

No files require special attention.

Important Files Changed

Filename Overview
src/Appwrite/Platform/Modules/Proxy/Action.php Single-character typo fix: empty($functionsDomains)empty($functionsDomain) so that empty entries from trailing/double commas in the env var are correctly skipped inside the loop.

Reviews (2): Last reviewed commit: "Fix domain validation checking wrong var..." | Re-trigger Greptile

The empty() check inside the forEach loop was checking the outer
$functionsDomains variable instead of the loop variable $functionsDomain,
causing empty entries to never be skipped when the env var is set.

Fixes appwrite#11675
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.

🐛 Bug Report: Proxy domain validation checks wrong variable in functions domain loop

1 participant