To reproduce, turn off Clean URLs and try to create a redirect from mysite.com/?q=node1&param1=1 to mysite.com/?q=node2 (where node1 and node2 are existing pages). Then navigate to mysite.com/?q=node1&param1=1. The redirect will not take place.

CommentFileSizeAuthor
#2 2968111-clean-url-querystrings.patch2.67 KBtomsegarra

Comments

tomsegarra created an issue. See original summary.

tomsegarra’s picture

tomsegarra commented
Assigned: tomsegarra » Unassigned
Status: Active » Needs review
StatusFileSize
new 2968111-clean-url-querystrings.patch2.67 KB

The problem is that parse_url() looks for a "?" character to separate the URL's path component from the querystring component. If Clean URLs are off, then the path is implicitly prefixed by "?q=". Since the "path" is then separated from the rest of the "querystring" by the "&" character rather than the "?" character, parse_url finds no querystring at all, thus parsing the URL incorrectly upon saving the redirect.

The attached patch adds a special case in redirect_parse_url(), delimiting the path and query components according to the "&" character if Clean URLs are found to be disabled.

It also adds a unit test for this function, which would fail without this code update, but should succeed after it's applied.

chris matthews’s picture

chris matthews commented
Status: Needs review » Reviewed & tested by the community

The 2 year old patch in #2 still applied cleanly to the latest 7.x-1.x-dev and fixes this issue for me so changing to RTBC.

git apply -v 2968111-clean-url-querystrings.patch
2968111-clean-url-querystrings.patch:10: trailing whitespace.
  // When Clean URLs are off, if a query component exists, it will be 
Checking patch redirect.module...
Hunk #1 succeeded at 1483 (offset 46 lines).
Checking patch redirect.test...
Hunk #1 succeeded at 160 (offset 27 lines).
Applied patch redirect.module cleanly.
Applied patch redirect.test cleanly.
warning: 1 line adds whitespace errors.
pifagor’s picture

pifagor commented

  • pifagor committed 03e1c8f on 7.x-2.x authored by tomsegarra 
    Issue #2968111 by tomsegarra, Chris Matthews, pifagor: Cannot create a...

  • pifagor committed 523778d on 7.x-1.x 
    Issue #2968111 by tomsegarra, Chris Matthews, pifagor: Cannot create a...
pifagor’s picture

pifagor commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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