[wp-trac] [WordPress Trac] #10126: Warning upon saving draft page

[WordPress Trac]

#10126: Warning upon saving draft page
-----------------------+----------------------------------------------------
  Reporter:  Beee      |        Type:  defect (bug)  
    Status:  reopened  |    Priority:  normal        
 Milestone:  3.0       |   Component:  Administration
   Version:  2.8       |    Severity:  normal        
Resolution:            |    Keywords:  needs-patch   
-----------------------+----------------------------------------------------
Changes (by dd32):

  * keywords:  has-patch => needs-patch


Comment:

 > what do you mean with "escaping will not help here"

 Quite simply, The error is BEFORE the database is brought into it, You can
 escape all you want, But the simple fact is, implode() is not being given
 an array, thus the Warning.

 > Warning: implode() [function.implode]: Invalid arguments passed in
 /home/username/domains/domainname.com/public_html/wp-includes/post.php on
 line 1762

 Well.. Looking at it closer, You're right it -could- be due to escaping,
 if someones Database class was not updated to support arrays in
 wpdb::escape() - Which was done in 2.8. Again, something WordPress cannot
 support.

 > and what about the loss of $post_ID and $post_parent? Those are passed
 as parameter to ->prepare():

 They're passed to prepare() but they're not used anywhere. You cant just
 pass params to prepare() and expect it to apply them to the query if there
 are no placeholders for them.

 The query will end up like this if under a default configuration
 {{{
 'SELECT post_name FROM $wpdb->posts WHERE post_name = %s AND post_type IN
 ('<POST_ID_HERE>');
 }}}

 > maybe hard to read because of having sprintf() and then ->prepare()
 Yes, It makes utterly no sense to use sprintf(), This is the common form
 in WordPress and is readable:
 {{{
 $check_sql = "SELECT post_name FROM $wpdb->posts WHERE post_name = %s AND
 post_type IN ($pattern)";
 }}}

 using sprintf to insert them into the phrase gives no benefit, other than
 making it hard for people to read.

 I'm all for converting it over to prepare(), But please test the
 function's output is the same as before, using the edge cases. The patches
 here will NOT pass that test - And will only work around the original
 problem, not solve it.

-- 
Ticket URL: <http://core.trac.wordpress.org/ticket/10126#comment:37>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software