Skip to content

fix: proxy route leading slash#874

Merged
oleksandr-nc merged 2 commits into
mainfrom
fix/proxy-route-leading-slash
May 15, 2026
Merged

fix: proxy route leading slash#874
oleksandr-nc merged 2 commits into
mainfrom
fix/proxy-route-leading-slash

Conversation

@oleksandr-nc
Copy link
Copy Markdown
Contributor

Restores route matching for ExApps that declare routes in the canonical regex form documented in HaRP (<url>^/path$</url>, e.g. app-skeleton-python).

ExAppProxyController::passesExAppProxyRoutesChecks builds a ^-anchored regex from $route['url'] and matches it against $other.
Symfony strips the leading slash, so $other is bare (e.g. public/page). But route URLs are typically written against the full path.

To avoid breaking ExApps whose info.xml still uses the non-canonical bare form (currently context_chat_backend's <url>downloadLogs</url>), the second commit adds a one-iteration fallback: if the canonical subject didn't match, retry against the bare subject and emit a debug log identifying the ExApp and route so maintainers know to migrate. Marked TODO(deprecation) for removal once known legacy apps have shipped the canonical form.

…ash (HaRP alignment)

Signed-off-by: Oleksander Piskun <oleksandr2088@icloud.com>
Signed-off-by: Oleksander Piskun <oleksandr2088@icloud.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 15, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: dbf0415e-dd07-4b7f-beea-d83316cfb692

📥 Commits

Reviewing files that changed from the base of the PR and between 43952c2 and 622083a.

📒 Files selected for processing (1)
  • lib/Controller/ExAppProxyController.php

📝 Walkthrough

Walkthrough

This PR updates route-matching behavior in passesExAppProxyRoutesChecks within the ExApp proxy controller. The matching logic now constructs a canonical subject by prefixing the request path with a leading slash before regex matching, aligning with HaRP's target_path semantics. If canonical matching fails, the code falls back to matching against the original route string and logs a debug message indicating the legacy path was used. This change affects which routes are considered eligible for subsequent verb and access-level checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: proxy route leading slash' directly and concisely summarizes the main change: fixing route matching by ensuring the leading slash is properly handled in proxy route comparisons.
Description check ✅ Passed The description clearly explains the problem, the solution, and the fallback mechanism for legacy routes, all directly related to the changeset addressing route matching in ExAppProxyController.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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


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

@oleksandr-nc oleksandr-nc merged commit f9445cb into main May 15, 2026
52 of 53 checks passed
@oleksandr-nc oleksandr-nc deleted the fix/proxy-route-leading-slash branch May 15, 2026 13:36
@oleksandr-nc
Copy link
Copy Markdown
Contributor Author

/backport to stable34

@oleksandr-nc
Copy link
Copy Markdown
Contributor Author

/backport to stable33

@oleksandr-nc
Copy link
Copy Markdown
Contributor Author

/backport to stable32

@oleksandr-nc
Copy link
Copy Markdown
Contributor Author

/backport to stable31

@oleksandr-nc
Copy link
Copy Markdown
Contributor Author

/backport to stable30

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