WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 6 years ago

#914 closed defect (bug) (fixed)

wrong search string escaping/slashes

Reported by: nbachiyski Owned by: ryan
Priority: normal Milestone: 2.1
Component: Template Version: 2.0.7
Severity: minor Keywords: has-patch commit
Cc: daniel@…

Description

Search for ' and
\' will appear in the input field.
" ->
\
& -> &

Attachments (2)

search-slashes.diff (2.2 KB) - added by nbachiyski 8 years ago.
914.diff (1.9 KB) - added by mdawaffe 7 years ago.
give it an template tag

Download all attachments as: .zip

Change History (21)

comment:1 nbachiyski8 years ago

  • Patch set to No

comment:2 nbachiyski8 years ago

In classes.php $qs? is added slashes again, despite the fact that is has passed through add_magic_quotes function before.

For database use the search string needs slashes, but when writing it to the templates is does not. I have added striptags calls in the template pages.

comment:3 ryan8 years ago

  • Patch changed from No to Yes

comment:4 ryan8 years ago

  • Owner changed from anonymous to rboren
  • Status changed from new to assigned

comment:5 ryan8 years ago

Even if we get rid of the extra addslashes, searches will still show a single set of slashes. \' instead of
\'. We can either add stripslashes to Kubrick's templates, or not addslashes by default when processing GPC in the blog header. Not adding slashes by default and instead relying on those functions that query the DB to addslashes as appropriate seems to be the cleanest way to do this, but that should wait until after 1.5.1.

comment:6 nbachiyski8 years ago

I also prefer not adding slashes by default and escape strings only for DB operations.

Now, as I understand, the choice is between leaving the bug in 1.5.1 or applying the dirty "stripslashes in Kubrick" hack before reorganizing all that code. My choice was the second.

Which is the less evil of the two?

nbachiyski8 years ago

comment:7 dwc8 years ago

  • Cc daniel@… added

mdawaffe7 years ago

give it an template tag

comment:8 mdawaffe7 years ago

  • Keywords has-patch commit added
  • Milestone set to 2.1

914.diff

  1. create wp_search_query() template tag which echos the query.

comment:9 ryan7 years ago

Whatcha think, wp_search_query() or the_search_query()? Or maybe just the_search()? These are important questions. :-)

comment:10 markjaquith7 years ago

the_search_query() or search_query()

the wp_blah() ones usually accept a query string with a bunch of parameters.

comment:11 mdawaffe7 years ago

I like the_search_query(), but search_query() is a much better band name.

comment:12 ryan7 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [4171]) the_search_query() from mdawaffe. fixes #914

comment:13 thenlich6 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened
  • Version changed from 1.5 to 2.0.7

Found the same problem in 2.0.7
' (single quote) becomes


\' with magic_quotes_gpc on (7 backslashes, then single quote),
or
\' (magic_quotes_gpc=off) (3 backslashes, quote)

comment:14 ryan6 years ago

With one of the default themes? If you're having problems with a third party theme, that theme needs to be changed.

comment:15 thenlich6 years ago

Yes, with theme: WordPress Default 1.6

comment:16 foolswisdom6 years ago

  • Milestone changed from 2.1 to 2.1.1

comment:17 markjaquith6 years ago

  • Resolution set to worksforme
  • Status changed from reopened to closed

thenlich, please upgrade to the most recent version of the theme (the one in 2.0.7). Re-open with a URL demonstrating the issue, if it persists.

comment:18 foolswisdom6 years ago

  • Milestone changed from 2.1.1 to 2.1
  • Resolution worksforme deleted
  • Status changed from closed to reopened

Changing back to previous fixed state.

comment:19 foolswisdom6 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed
Note: See TracTickets for help on using tickets.