WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 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 3 years ago.
16548.diff (1.2 KB) - added by dd32 3 years ago.
18148.patch (704 bytes) - added by SergeyBiryukov 3 years ago.
The patch from #18148 without unnecessary urldecode()

Download all attachments as: .zip

Change History (17)

comment:1 solarissmoke3 years ago

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

SergeyBiryukov3 years ago

comment:2 SergeyBiryukov3 years ago

  • Keywords has-patch added

comment:3 solarissmoke3 years ago

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

comment:4 SergeyBiryukov3 years ago

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

comment:5 follow-up: dd323 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.

dd323 years ago

comment:6 dd323 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: scribu3 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 solarissmoke3 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 scribu3 years ago

dd32's patch works.

comment:10 ryan3 years ago

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

Per bug scrub, punting to 3.2.

comment:11 kawauso3 years ago

  • Keywords 3.3-early added; 3.2-early removed

Per chat with westi

comment:12 in reply to: ↑ 5 SergeyBiryukov3 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 nacin3 years ago

  • Milestone changed from Future Release to 3.3

SergeyBiryukov3 years ago

The patch from #18148 without unnecessary urldecode()

comment:14 ryan3 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.