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.

CommentFileSizeAuthor
#10 remove-spaces-from-path-3032976-10.patch785 bytesgcb
#4 remove-spaces-from-path-3032976-2.patch795 bytesmiteshmap
#2 remove-spaces-from-path-3032976-2.patch787 bytesmiteshmap

Comments

miteshmap created an issue. See original summary.

miteshmap’s picture

miteshmap
Primary language Gujarati
commented
Assigned: miteshmap » Unassigned
Status: Active » Needs review
StatusFileSize
new remove-spaces-from-path-3032976-2.patch787 bytes

patch for 8.x-1.x

miteshmap’s picture

miteshmap
Primary language Gujarati
commented
Title: Remove leading or tailing spaces from source url » Remove tailing spaces from source url
miteshmap’s picture

miteshmap
Primary language Gujarati
commented
StatusFileSize
new remove-spaces-from-path-3032976-2.patch795 bytes

Status: Needs review » Needs work

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

romixua’s picture

romixua
Location 🇺🇦 lviv
commented
Assigned: Unassigned » romixua
Issue tags: +epam-contrib-2019.03
sosevich.v’s picture

sosevich.v commented
Status: Needs work » Needs review

This patch applies on my side. Needs review again.

rodman1980’s picture

rodman1980 commented
Issue tags: -epam-contrib-2019.03
romixua’s picture

romixua
Location 🇺🇦 lviv
commented
Assigned: romixua » Unassigned
gcb’s picture

gcb
he/him
Location Portland, OR
commented
Version: 8.x-1.3 » 8.x-1.4
StatusFileSize
new remove-spaces-from-path-3032976-10.patch785 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

sosevich.v commented
Status: Needs review » Reviewed & tested by the community

Lets have it merged finally!

gcb’s picture

gcb
he/him
Location Portland, OR
commented
Title: Remove tailing spaces from source url » Remove trailing spaces from source url
dinesh18’s picture

dinesh18 commented

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

berdir
Primary language German
Location Switzerland
commented
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

bkosborne
Primary language English
Location New Jersey, USA
commented

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.