Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

IBX-5821: Fixed an issue where incomplete request object was passed over to route Matcher #319

Merged
merged 3 commits into from
Jan 30, 2024

Conversation

Steveb-p
Copy link
Contributor

Question Answer
JIRA issue IBX-5821
Type bug
Target Ibexa version v4.5
BC breaks no

This PR fixes an issue where an incomplete Request object was passed into UrlMatcher, because old Router::match() method was called internally when Router::matchRequest() was used.

New code mimics the old behavior, but instead of creating an entirely new Request object for matching, it duplicates the existing one (using a built-in method) and replaces REQUEST_URI with new path value (the same as used before).

I can confirm that both Admin UI and basic front works with this solution.

image

image

Tests were updated to more specifically check what values are used by Matcher.

Checklist:

  • Provided PR description.
  • Tested the solution manually.
  • Provided automated test coverage.
  • Checked that target branch is set correctly (main for features, the oldest supported for bugs).
  • Ran PHP CS Fixer for new PHP code (use $ composer fix-cs).
  • Asked for a review (ping @ibexa/engineering).

@Steveb-p Steveb-p requested a review from a team January 10, 2024 10:59
@Steveb-p Steveb-p added Bug Something isn't working Ready for review labels Jan 10, 2024
Copy link

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@alongosz alongosz requested a review from a team January 10, 2024 14:12
Copy link
Contributor

@mnocon mnocon left a comment

Choose a reason for hiding this comment

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

I've verified that the scenario described in the ticket now works (with a conditionally matcher route). I've also performed basic sanity checks using frontend and admin siteaccesses (using URIElement and Map\Host matchers).

Looks good, QA approved!

@adamwojs adamwojs merged commit 9150cc6 into 4.5 Jan 30, 2024
21 checks passed
@adamwojs adamwojs deleted the ibx-5821/fix-router-using-only-path-info branch January 30, 2024 15:45
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants