Properly preparing data in query for $wpdb WordPress
- SOLVED
Hi,
I'm trying to resolve an issue related to properly preparing data in a query for for $wpdb. I've had some difficulty getting this to work properly and would like some direction, if possible.
The query in question is:
<?php
global $wpdb;
$images = get_post_meta( get_the_ID(), 'siiimple_gallery', false );
$images_ids = implode( ',' , $images );
$query = "SELECT ID FROM $wpdb->posts WHERE post_type = %s AND ID IN (".$images_ids.") ORDER BY menu_order ASC";
$images = $wpdb->get_col($wpdb->prepare( $query ,'attachment'));
$test = $wpdb->last_query;
foreach ( $images as $attachment_ID ) {
$attachment_url = wp_get_attachment_url($attachment_ID , 'full');
$attachment = aq_resize($attachment_url, 840, 650, true, true, true);
echo '<div class="slick-item"><img src="' . $attachment . '" class="img-responsive" alt=""/></div>';
}
?>
It is a simple slider query and I've added ->prepare where I thought was the correct place but I'm being instructed that it is incorrect and the reviewer added the screenshot below:
http://envato.d.pr/1j29M/5xpnnadj
Thanks for any help!
Justin
Answers (2)
Dbranes answers:
Part of the problem is that the user input in <em>$images</em> isn't escaped.
Replace:
$images_ids = implode( ',' , $images );
with:
$images_ids = join( ',', array_map( 'absint', $images ) );
Then you should makes some checks to see if <em>$images_ids</em> is empty.
But why don't you use <em>WP_Query </em>or <em>get_posts </em>?
<strong>Update:</strong>
Here's an untested rewrite example:
if( function_exists( 'aq_resize' ) ) {
$images = get_post_meta( get_the_ID(), 'siiimple_gallery', false );
$images_ids = array_map( 'absint', $images );
if( ! empty( $images_id ) ){
$args = array(
'post_type' => 'attachment',
'post_status' => 'any',
'post__in' => (array) $images_ids,
'orderby' => 'menu_order',
'order' => 'ASC',
'fields' => 'ids',
);
$images = get_posts( $args );
foreach ( (array) $images as $attachment_ID ) {
$attachment_url = wp_get_attachment_url( $attachment_ID , 'full' );
$attachment = aq_resize( $attachment_url, 840, 650, true, true, true );
printf( '<div class="slick-item"><img src="%s" class="img-responsive" alt=""/></div>', $attachment );
}
} // end if $images_id isn't empty
} // end if function aq_rezise exists
Justin Young comments:
It's grabbing attachment images for a gallery post. So the full query looks like this:
<?php
$i = 1;
if ( get_query_var('paged') ) {
$paged = get_query_var('paged');
} elseif ( get_query_var('page') ) {
$paged = get_query_var('page');
} else {
$paged = 1;
}
query_posts(
array(
'showposts' => $siiimple_options['latest_num'],
'cat' => implode(",", $siiimple_options['latest']),
'paged' => $paged ) );
if ( have_posts() ) : $count = 0; while ( have_posts() ) : the_post(); $count++;
<div class="slick-slider">
<?php
global $wpdb;
$images = get_post_meta( get_the_ID(), 'siiimple_gallery', false );
$images_ids = join( ',', array_map( 'absint', $images ) );
$query = "SELECT ID FROM $wpdb->posts WHERE post_type = %s AND ID IN (".$images_ids.") ORDER BY menu_order ASC";
$images = $wpdb->get_col($wpdb->prepare( $query ,'attachment'));
$test = $wpdb->last_query;
foreach ( $images as $attachment_ID ) {
$attachment_url = wp_get_attachment_url($attachment_ID , 'full');
$attachment = aq_resize($attachment_url, 800, 500, true, true, true);
echo '<div class="slick-item"><img src="' . $attachment . '" class="img-responsive" alt=""/></div>';
}
?>
<p class="cat small"><?php echo siiimple_cat(); ?></p>
<h2><a href="<?php the_permalink(); ?>"><?php the_title(); ?></a></h2>
<p><?php echo excerpt('20'); ?></p>
<p class="human-time"><?php echo human_time_diff( get_the_time('U'), current_time('timestamp') ) ?> / <?php comments_popup_link( __( '<span class="human-comments">0 Comments</span>', 'siiimple' ), __( '<span class="human-comments">1 Comment</span>', 'siiimple' ), __( '<span class="human-comments">% Comments</span>', 'siiimple' ) ); ?></p>
</div>
<?php $i++; endwhile; ?>
<?php endif; ?>
I replaced the text you mentioned and it still works with no errors.
Justin Young comments:
The re-write doesn't throw up any errors but the images don't show. Checked the source code and it doesn't show the images.
Dbranes comments:
You should also avoid using <em>query_post</em>.
Here's a note from the Codex:
<blockquote>Note: This function isn't meant to be used by plugins or themes. As explained later, there are better, more performant options to alter the main query. query_posts() is overly simplistic and problematic way to modify main query of a page by replacing it with new instance of the query. It is inefficient (re-runs SQL queries) and will outright fail in some circumstances (especially often when dealing with posts pagination). Any modern WP code should use more reliable methods, like making use of pre_get_posts hook, for this purpose.</blockquote>
Dbranes comments:
ps: <em>post__in</em> must accept an array, so I removed the join part in the rewrite example above.
