WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#16548 closed defect (bug) (fixed)

Plugin searches drop spaces with further action

Reported by: kawauso Owned by: ryan
Milestone: 3.3 Priority: normal
Severity: normal Version: 3.1
Component: Administration Keywords: has-patch commit 3.3-early
Focuses: Cc:

Description

Steps to reproduce:

  • Search for a valid plugin name with a space in it
  • Activate it
  • The space will be dropped from the redirected URL by wp_redirect() sanitization

Attachments (3)

16548.patch (756 bytes) - added by SergeyBiryukov 4 years ago.
16548.diff (1.2 KB) - added by dd32 4 years ago.
18148.patch (704 bytes) - added by SergeyBiryukov 4 years ago.
The patch from #18148 without unnecessary urldecode()

Download all attachments as: .zip

Change History (17)

comment:1 @solarissmoke4 years ago

Maybe we should drop the search query var when doing other actions?

@SergeyBiryukov4 years ago

comment:2 @SergeyBiryukov4 years ago

  • Keywords has-patch added

comment:3 @solarissmoke4 years ago

@SergeyBiryukov the patch doesn't change anything? Steps to reproduce still cause the same issue.

comment:4 @SergeyBiryukov4 years ago

Ah, I was checking the wrong installation. Feel free to make a proper one :)

comment:5 follow-up: @dd324 years ago

This comes back to the usage of "...&s=$s" which translates to "...&s=Add From Server" which is obviously wrong.

Given the sheer number of redirects it's used in, This patch simply urlencodes it when it gets it from the request.

@dd324 years ago

comment:6 @dd324 years ago

  • Keywords commit added
  • Milestone changed from Awaiting Review to 3.1

Bumping to 3.1 for another dev to check

comment:7 follow-up: @scribu4 years ago

Search for a valid plugin name with a space in it

I assume we're talking about the plugin's directory name, not the "Plugin Name:" header.

Also, is this a regression?

comment:8 in reply to: ↑ 7 @solarissmoke4 years ago

Replying to scribu:

I assume we're talking about the plugin's directory name, not the "Plugin Name:" header.

No, we are talking about the plugin name, e.g. "Hello Dolly"

Also, is this a regression?

No - it's actually worse in 3.0.x because the search query is emptied, and you get this notice:

Notice: Undefined variable: total_search_plugins in wp-admin\plugins.php on line 475 

comment:9 @scribu4 years ago

dd32's patch works.

comment:10 @ryan4 years ago

  • Keywords 3.2-early added
  • Milestone changed from 3.1 to Future Release

Per bug scrub, punting to 3.2.

comment:11 @kawauso4 years ago

  • Keywords 3.3-early added; 3.2-early removed

Per chat with westi

comment:12 in reply to: ↑ 5 @SergeyBiryukov4 years ago

Replying to dd32:

Given the sheer number of redirects it's used in, This patch simply urlencodes it when it gets it from the request.

Looks like urldecode() for display is not required, since $s is restored to non-encoded form in wp_reset_vars() via $wp_list_table->prepare_items().

comment:13 @nacin4 years ago

  • Milestone changed from Future Release to 3.3

@SergeyBiryukov4 years ago

The patch from #18148 without unnecessary urldecode()

comment:14 @ryan4 years ago

  • Owner set to ryan
  • Resolution set to fixed
  • Status changed from new to closed

In [18452]:

Fix redirects for plugin searches containing spaces in the search string. Props SergeyBiryukov. fixes #16548

Note: See TracTickets for help on using tickets.