Fix redirect encoding

[jonasdekeukelaere]

https://next-app.activecollab.com/108877/my-work?modal=Task-253942-4609

Summary by Sourcery

Adjust redirect URL encoding to correctly handle quotes while preserving query strings and fragments.

Bug Fixes:

• Fix redirect URL encoding so that only quotes in the path are percent-encoded without breaking query parameters or fragments.

Tests:

• Extend redirect URL encoding tests to cover URLs containing fragments with non-ASCII characters.

[sourcery-ai]

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Refactors redirect URL encoding to use parse_url and targeted quote encoding while preserving fragments and queries, and extends tests to cover URLs with fragments containing UTF-8 characters.

Flow diagram for updated getEncodedRedirectUrl encoding logic

flowchart TD
    A[Input redirectUrl] --> B[parse_url redirectUrl]
    B --> C{parsedUrl path set?}
    C -- yes --> D[str_replace quotes in parsedUrl path
        with %22 and %27]
    C -- no --> E[Leave path as null]
    D --> F[Build base URL:
        scheme://host + path]
    E --> F
    F --> G{parsedUrl query set?}
    G -- yes --> H[Append ?query]
    G -- no --> I[Skip query]
    H --> J{parsedUrl fragment set?}
    I --> J
    J -- yes --> K[Append #fragment]
    J -- no --> L[Skip fragment]
    K --> M[Return url]
    L --> M[Return url]

Loading

File-Level Changes

Change	Details	Files
Refactor redirect URL encoding logic to use URL parsing and targeted path encoding instead of manual host/path splitting and rawurlencode on segments.	Replace regex-based extraction and explode/implode logic with parse_url to split scheme, host, path, query, and fragment. Encode only double and single quotes in the URL path as %22 and %27, leaving other UTF-8 characters untouched. Reconstruct the URL from parsed components, conditionally appending query string and fragment if present. Return the newly constructed URL string instead of the original input.	src/Backend/Modules/Pages/Engine/Model.php
Extend unit test coverage for encoded redirect URLs to ensure fragments and UTF-8 characters are preserved correctly.	Keep existing assertion for encoding quotes in the URL path unchanged. Add a new assertion verifying that a URL containing a UTF-8 fragment identifier is returned without unwanted encoding or modification.	src/Backend/Modules/Pages/Tests/Model/ModelTest.php

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• The new getEncodedRedirectUrl implementation assumes parse_url always returns scheme, host, and path, which will trigger notices or malformed URLs for inputs like relative URLs, scheme-less URLs, or URLs without a path; consider adding guards for parse_url === false and missing components before concatenation.
• The reconstructed URL currently drops other possible URL parts such as port, user, and pass; if those are valid inputs in this context, you may want to include them when rebuilding the URL.
• Directly concatenating $parsedUrl['path'] without defaulting to an empty string can produce an undefined index notice when the original URL has no path (e.g., http://example.com); consider using $parsedUrl['path'] ?? '' in the concatenation.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The new `getEncodedRedirectUrl` implementation assumes `parse_url` always returns `scheme`, `host`, and `path`, which will trigger notices or malformed URLs for inputs like relative URLs, scheme-less URLs, or URLs without a path; consider adding guards for `parse_url === false` and missing components before concatenation.
- The reconstructed URL currently drops other possible URL parts such as `port`, `user`, and `pass`; if those are valid inputs in this context, you may want to include them when rebuilding the URL.
- Directly concatenating `$parsedUrl['path']` without defaulting to an empty string can produce an undefined index notice when the original URL has no path (e.g., `http://example.com`); consider using `$parsedUrl['path'] ?? ''` in the concatenation.

## Individual Comments

### Comment 1
<location path="src/Backend/Modules/Pages/Engine/Model.php" line_range="1556-1560" />
<code_context>
+            $parsedUrl['path'] = str_replace(['"', '\''], ['%22', '%27'], $parsedUrl['path']);
+        }
+
+        $url = $parsedUrl['scheme'] . '://' . $parsedUrl['host'] . $parsedUrl['path'];
+        if (isset($parsedUrl['query'])) {
+            $url .= '?' . $parsedUrl['query'];
+        }
+        if (isset($parsedUrl['fragment'])) {
+            $url .= '#' . $parsedUrl['fragment'];
         }
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Preserve optional URL components like port, user/pass and guard against missing path.

This reconstruction omits `port`, `user`, `pass`, and assumes `path` is always set. For example, `https://user:pass@example.com:8443/foo` would lose credentials and port, and URLs without a path would hit an undefined index on `$parsedUrl['path']`. Please build `$url` conditionally from all present components (user/pass, host, port, path), and treat a missing path as an empty string.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}