#6836 closed defect (bug) (fixed)
Prepare queries using INSERT/UPDATE should use wpdb::insert() or wpdb::update()
Reported by: | DD32 | Owned by: | ryan |
---|---|---|---|
Milestone: | 3.0 | Priority: | normal |
Severity: | normal | Version: | 2.6 |
Component: | Administration | Keywords: | has-patch |
Focuses: | Cc: |
Attachments (13)
Change History (39)
#2
@
17 years ago
I think the best way to do it is one file at a time.
Yep, I agree with that. I might make a start over the weekend on some of them.
#3
@
17 years ago
attachment 6836.diff added.
Note: There was a escaping issue with adding meta/updating meta, ' and " were being double escaped. Patch as such removes the double escpaping.
Also renamed a few variables so as to use compact('meta_value') instead of array('meta_value' => $metavalue,..). Just makes it look a bit more clean and keeps fields named the same throughout.
All Meta changes were tested working, The last chunk(Change to _relocate_children()
) hasnt been tested, But i see no reason it should fail.
@
17 years ago
Tested with a New Install only; Upgrade functions not tested, Could do with an extra eye-over
#4
@
17 years ago
Just noticed this: http://trac.wordpress.org/browser/trunk/wp-includes/functions.php#L527
532: $type = $wpdb->escape( $headers['content-type'] ); }} I assume it was probably left over from when the raw query was there.. removed it in 6836.5-2.diff
#6
@
16 years ago
About to add a full refreshed patch.
I've been running my local install patched like this for awhile, and not had any issues with escaped data.
#8
follow-up:
↓ 9
@
16 years ago
insert() and update() don't do prepare(). I would like to pass format specifiers to them somehow before moving everything over.
#9
in reply to:
↑ 8
@
16 years ago
Replying to ryan:
insert() and update() don't do prepare(). I would like to pass format specifiers to them somehow before moving everything over.
How about something similar to this:
insert ('test_table', array('ID' => 's', 'post' => 'test'), array('ID' => '%d')); creating: INSERT INTO test_table (`ID`,`post`) VALUES ('0','test')
(See patch)
#10
@
16 years ago
that could be changed to this to make it more compact:
foreach( (array)$types as $key => $type ) if( isset($data[$key]) ) $data[$key] = sprintf($type, $data[$key]);
@
16 years ago
My Patch to marks/annotates all uses of inline SQL outside of wp-db. This patch includes the cases where a raw SQL update/insert is used instead of the existing methods in wp-db (search for "@RawSQLUse, method_exists"). Also see the discussion on wp-hackers (search for "Making WP more secure the evolutionary way").
@
16 years ago
Simple annotation version to mark all the remaining raw uses of INSERT/UPDATE (+simple cases of DELETE and SELECT)
#16
@
15 years ago
- Keywords needs-patch added; has-patch needs-testing removed
guess not. there are still a few $wpdb->query("INSERT ... and $wpdb->query("UPDATE ... around.
#19
@
15 years ago
- Keywords has-patch added; needs-patch removed
- Milestone changed from Future Release to 3.0
attachment 6836.6.diff added
Adds the final few UPDATE/INSERT conversions that are not using complex SQL.
Most of the last areas that use it is wp-admin/includes/upgrade.php
which i've been avoiding as i'm not going to be able to test the changes properly there.. So i'm tempted to leave that code be for now while its in working condition, Its prepare()'d, just not update()'d
#20
@
15 years ago
attachment 6836.7.diff added
- Removed a useless => %d specification on a query
- $wpdb->field_types takes care of those fields.
#23
@
15 years ago
I don't get ryan's last comment. Does it mean this ticket is partially fixed and some queries in upgrade related functions were not changed? If so, which were they?
#24
@
15 years ago
Those from wp-admin/includes/upgrade.php (6836.3.diff )?
#25
@
15 years ago
Does it mean this ticket is partially fixed and some queries in upgrade related functions were not changed? If so, which were they?
It means any changes to the upgrade scripts in the supplied patch were not commited, And will not be commited.
The only reason is that testing DB changes made to the upgrade scripts is a serious undertaking, Its hard to test every case of every SQL branch, the current implementation is working and uses proper escaping, so its best left running as-is.
Cheers! I think the best way to do it is one file at a time. That way a lot of people can participate without having to all share one big monster diff.