If the source url contains a trailing space, it is not being stripped. Making it impossible to redirect to the given destination url.

For example, if we add a link like "/some/random/path ", with spaces. The source url should set as "/some/random/path", but the module keeps it as is.

But there's also a scenario which we need to consider here, Which is raised at https://www.drupal.org/project/redirect/issues/2318079 To keep the leading space as is.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

miteshmap created an issue. See original summary.

miteshmap’s picture

Assigned: miteshmap » Unassigned
Status: Active » Needs review
FileSize
787 bytes

patch for 8.x-1.x

miteshmap’s picture

Title: Remove leading or tailing spaces from source url » Remove tailing spaces from source url
miteshmap’s picture

Status: Needs review » Needs work

The last submitted patch, 4: remove-spaces-from-path-3032976-2.patch, failed testing. View results

Romixua’s picture

Assigned: Unassigned » Romixua
Issue tags: +epam-contrib-2019.03
sosevich.v’s picture

Status: Needs work » Needs review

This patch applies on my side. Needs review again.

rodman1980’s picture

Issue tags: -epam-contrib-2019.03
Romixua’s picture

Assigned: Romixua » Unassigned
gcb’s picture

Version: 8.x-1.3 » 8.x-1.4
FileSize
785 bytes

The array formatting for the lines before and after this patch were updated to the more concise version. Here's a re-roll that should apply. This issue exists in 1.4 as well, so bumping the version to latest.

sosevich.v’s picture

Status: Needs review » Reviewed & tested by the community

Lets have it merged finally!

gcb’s picture

Title: Remove tailing spaces from source url » Remove trailing spaces from source url
Dinesh18’s picture

patch #10 works perfectly fine. +1 to RTBC

  • Berdir committed 67f6f77 on 8.x-1.x authored by gcb
    Issue #3032976 by miteshmap, gcb: Remove trailing spaces from source url
    
Berdir’s picture

Status: Reviewed & tested by the community » Fixed

Committed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

bkosborne’s picture

I do not think this was a great decision to have leading spaces kept. I outlined my reasoning in #2318079: Leading space is being stripped from "From" path; proposed fix:

Think of it this way. What percentage of users do you think have a valid need to create a redirect with a leading space compared to the percentage of users that are copying and pasting "from" paths which unintentionally have a leading space? I think the latter is FAR more likely and something I encounter as a site admin many times per year. The case that this issue is describing seems very unlikely to happen, and I think a different solution should be provided.

At the very least, if this is implemented, and a leading space is detected, then a prominent warning should be provided to the editor to let them know they pasted in a leading space.

I'll open a new issue to ask that this be changed to a normal trim instead of rtrim.