WordPress.org

Support

Support » Plugins and Hacks » [Resolved] Some fixes needed to work with actual WP version

[Resolved] Some fixes needed to work with actual WP version

  • Hi there,

    I fixed this live notifications with two simple steps, and now it works.

    1. Replace enclosed quotes in server response (see file bpln.js from line 20):


    var d2 = data.replace(/^\"(.*?)\"$/, "'$1'");
    data=JSON.parse(d2);

    2. Add NULL argument to function call in file bp-live-notification.php at line 152:


    $notifications =$wpdb->get_results( $wpdb->prepare( "SELECT * FROM {$bp->core->table_name_notifications} WHERE id in {$list_ids} AND is_new = %d", NULL), 1 );

    Optionally you can add “alert” class to notification (bp-live-notification.php, line 121:


    $menu_title = sprintf( __( 'Notifications <span id="ab-pending-notifications" class="pending-count alert">%s</span>', 'buddypress' ), count( $notifications ) );

    https://wordpress.org/plugins/bp-live-notification/

Viewing 10 replies - 1 through 10 (of 10 total)
  • Plugin Author Brajesh Singh
    Member

    @sbrajesh

    Hi Alex,
    I am sorry for the inconvenience. but your proposed answers are not the solution.
    1. Can you please tell me why do we need to do this

    var d2 = data.replace(/^\"(.*?)\"$/, "'$1'");
    data=JSON.parse(d2);

    when the data sent from server is already in JSON format?
    2. What is the purpose of passing null when we are checking for new notification?

    Thank You
    Brajesh

    Hi, Brajesh.

    1. Server response is a string and therefore it is parsed as "somedata", e.g. "{"a":"1"}". So we have JSON syntax error there.

    2. Second argument in function $wpdb->get_results is NOT optional.

    Thanks for plugin.

    I’m sorry, I mean function $wpdb->prepare (file wp-includes/wp-db.php at line 1147):

    function prepare( $query, $args )

    Plugin Author Brajesh Singh
    Member

    @sbrajesh

    Hi Alex,
    instead of using the Js mod, I added the 4th parameter as ‘json’. So, we don’t need to even parse that on client side anymore.

    Can you please check if the new upgrade works for you.

    There is a description of problem with prepare function: PHP Warning: Missing argument 2 for wpdb::prepare() and there is subject: Protect Queries Against SQL Injection Attacks.

    So actually fix for problem is (if you don’t suppress error_reporting):

    $notifications =$wpdb->get_results( $wpdb->prepare( "SELECT * FROM {$bp->core->table_name_notifications} WHERE id in {$list_ids} AND is_new = %d", $notification_ids), 1 );

    Sorry again for NULL (this is a dirty hack), but I’m not a WP developer.

    Plugin Author Brajesh Singh
    Member

    @sbrajesh

    Hi Alex,
    I understand that. That’s why I said about your last update. We don’t need to modify the query. It was related to JSON proper parsing. Please upgrade to the 1.0.5 and let me know if that works for you or not.

    Hi Brajesh,

    it works, thanks (I forgot about response format in jQuery)!

    You said: “We don’t need to modify the query”, but added parameter does not modify query, in your actual version it looks like:

    $wpdb->prepare( "SELECT * FROM {$bp->core->table_name_notifications} WHERE id in {$list_ids} AND is_new = %d", 1)

    I don’t know how WP handles queries, I’m completely WP newbie, but why $wpdb->prepare works with any argument in this case, even NULL (and with $notification_ids — it’s my mistake, sorry, actually $list_ids is needed)? As a result we have insecure query, doesn’t?

    Plugin Author Brajesh Singh
    Member

    @sbrajesh

    Hi,
    Thank you for confirming.
    List ids do not make the query insecure. It is not user generated, It is coming from database and that too is independent of user generated content. So, No that does not make the query insecure.

    Brajesh,

    thanks for support! I proposed also add “alert” class (defined in buddypress/bp-core/css/admin-bar.css) to notifications counter, but it’s just a little cosmetic improvement:

    View post on imgur.com

    Plugin Author Brajesh Singh
    Member

    @sbrajesh

    Hi Alex,
    Thank you.
    I am sorry I did not add the class alert as it is too generic and may cause conflict on some theme. I am sure you can style it using available selectors.

Viewing 10 replies - 1 through 10 (of 10 total)
  • The topic ‘[Resolved] Some fixes needed to work with actual WP version’ is closed to new replies.