WordPress.org

Ready to get started?Download WordPress

Forums

$wpdb (28 posts)

  1. Kuckovic
    Member
    Posted 1 year ago #

    Hi everyone!

    I'm developing a plugin, and I'm almost done.
    I have some issues though.

    When I add something to my database, it's shown on my admin-dashboard. That part works fine and dandy! But! I want a "Delete" button to each "item"..

    This is how i "Call" the items from my DB:

    global $wpdb;
    $table_name = $wpdb->prefix."comment_reminder";
    $cremindsql = $wpdb->get_results("SELECT * FROM $table_name ORDER BY id DESC");

    As I said, that part works great!
    Now, I want to add a DELETE button - I just can't figure out how I do this. I have this code for now:
    $delblog = $wpdb->query("DELETE FROM $table_name WHERE id ='1');

    How can I make "WHERE id = '1'" to be automatic?
    I mean, I have a button beside each item, not I need, somehow, to put an "id" to each button?

    How do I do this? :(

    Thanks
    Kucko

  2. bcworkz
    Member
    Posted 1 year ago #

    You assign the id to a variable, then use it like this:
    "...WHERE id = '$id'"
    So you need a way to get the id for the item. It could be passed as an url parameter in a delete link. Or in a hidden field to be passed as part of a form POST.

    Be sure to use a nonce as part of the request too so no one can fabricate a GET or POST request to do the same without credentials.

  3. Kuckovic
    Member
    Posted 1 year ago #

    Hi bcworkz,

    This is what my form looks like:

    <form method="post" action="options.php">
        <label>Blog URL</label>
        <input type="text" id="cr_url" name="cr_url">
        <label>Blog Name</label>
        <input type="text" id="cr_name" name="cr_name"><br><br>
        <input type="submit" class="button-primary" value="Save info" />
        </form>

    So now, I have to put ID in a hidden field you say?
    Or I can use a Delete link.¨¨

    I more interested in the delete link - but I really dont know how. Can you help me out a bit? :)

  4. bcworkz
    Member
    Posted 1 year ago #

    I agree a link makes more sense than a form. When you are outputting the html for each item, I assume the item id is available in php code? So you could build a delete link who's href might look like example.com/handlerpage.php?action=del&id=372 . You should pass this through wp_nonce_url() before echoing out or anyone could delete something by typing the link into their browser.

    The handler page receiving the request would get the parameters from the $_GET array. You would use check_admin_referer(); to verify the nonce is valid, then go ahead and do the delete query using the passed id value. If successful, some sort of verification should probably be sent to the user, or an error message on any failure.

    Yet another possibility is using ajax to send the request and ID, then the response can be reflected right on the current page without need to reload. There's info in the Codex on how to do this, but it's not that thorough. Various tutorials outside of wordpress.org are your best bet. If you get stuck, come back here.

  5. Kuckovic
    Member
    Posted 1 year ago #

    It WORKS!
    Thanks A LOT bcworkz!

    I created a delete.php and now it works!
    If anyone else needs help, read this - it worked for me!

    http://www.phpsimple.net/mysql_delete_record.html

  6. Kuckovic
    Member
    Posted 1 year ago #

    Oh I have another question.
    WordPress.org tells me I can't include wp-config
    That there's other ways to do it - this is what they write:

    It's best if you tie your processing functions (the ones that need but don't have access to core functions) into an action hook, such as "init" or "admin_init".

    Can anyone help me out here?

  7. bcworkz
    Member
    Posted 1 year ago #

    That doesn't make sense to me, you need access to core functions to add an action hook, how can it be done if you don't have access to core functions? (Rhetorical question, no answer expected.)

    Why were you including wp-config? You need access to the $wpdb global from your delete.php page? In most cases you should require_once( $path . 'wp-load.php' ); . But since you would want the user doing the deleting to be logged in to delete records, I assume, you should require wp-admin/admin.php instead. This does not mean the user needs to be admin, just that they need to be logged in.

  8. Kuckovic
    Member
    Posted 1 year ago #

    This is what my delete.php looks like:

    <?php
    
    require_once("../../../wp-config.php");
    global $wpdb;
    
    $table_name = $wpdb->prefix."comment_reminder";
    
    $blogid=$_GET['id'];
    
    $wpdb->query( $wpdb->query( "DELETE FROM $table_name WHERE id='$blogid'" ) );
    
    header("location:/wp-admin/options-general.php?page=comment-reminder");
    
    ?>

    I really can't figure out how to do it else.
    It works perfectly when delete.php is like this.

  9. bcworkz
    Member
    Posted 1 year ago #

    OK, there's two problems with that, one's minor, the other is a significant security flaw.

    To fix the minor one, simply replace wp-config.php with wp-load.php. If you look at wp-load.php, all it really does is include wp-config.php, so you were pretty close with what you have. But it also does some important checks, so it's worth doing it right.

    Left with that easy fix, anyone could type a delete request into their browser and your script will happily delete the data without question. Even worse, you don't check if the id is a sane value, you are wide open to an SQL injection attack. Through a series of exploits, your site could end up fully compromised, in other words, hacked.

    At the very least, require wp-admin/admin.php instead of wp-load.php or wp-config.php. At least then someone would have to be logged in to delete data, but it could be anyone logged in, even a common user with no capabilities. You still must verify the id value passed is a sane value, such as it must be a positive integer less than 32768.

    You really should also verify the user has the proper capability assigned to do this action. Finally, the link should include a nonce which the delete.php script verifies so you are assured the request came from a validly served page and is not some sort of attack.

    All these layers of security may seem paranoid. Try putting 'hacked' into this site's search box. You'll get over 15000 results, virtually all support requests for hacked sites. I've personally responded to dozens of these, and am frankly rather tired of it. Don't be another victim.

  10. Kuckovic
    Member
    Posted 1 year ago #

    Okay, so I will require admin.php instead og wp-config.

    Can you please tell me how to do the rest?
    Im really rabbish at MySQL and so, but I've "invented" this plugin for a personal use, but I would also like to share it.

    I have a lot to learn I see.
    But if I get the right guidance, I could learn it myself later on, because then I have a "protocol" to look at.

    You're talking about a sane value - how do I do that?
    And the "nonce" you're talking about?

    I can paste my codes here, if you want?
    And also, I will "credit" you in the plugin for your help :)

  11. bcworkz
    Member
    Posted 1 year ago #

    Sane Values: In this case for an ID, just checking that it is indeed an integer in a certain range is enough. For other situations, it depends on the expected data. You want to restrict the value as much as possible without constricting any valid input. Also take a look at Validating Sanitizing and Escaping User Data.

    Nonces: Nonce is a portmanteau of Number used ONCE. Specifically for WordPress, it is a unique, large, hexadecimal number that a server attaches to a form or link. When the server receives a request purportedly from that form or link, it can look at this number to correlate it to which form or link it sent out to begin with, because each nonce is used only once.

    Thus, if the nonce received does not correlate to one that was recently sent, it can safely be assumed the request is invalid and possibly some sort of hack attempt, and the request can be ignored. Acceptance of any request that comes in that would result in a change of the current state of the server or database should be protected with a nonce. Start with WordPress Nonces for more on nonces.

    Once you've developed reasonable security measures, I would be happy to review the code and give you some feedback if you desire. Crediting my assistance is hardly necessary, but I would of course be flattered if you chose to do so.

  12. Kuckovic
    Member
    Posted 1 year ago #

    Hi bcworkz!

    Now I've looked around on the internet, to try and find out more about "nonces" and "admin_referrer" - and I've come up with a solution.

    This is how my Form looks like now:

    <form method="post" action="options.php">
        <label>Blog URL</label>
        <input type="text" id="cr_url" name="cr_url">
        <label>Blog Name</label>
        <input type="text" id="cr_name" name="cr_name"><br><br>
        <input type="submit" class="button-primary" value="Save info" />
        <?php wp_nonce_field('verify_creminder','creminder_nonce'); ?>
        </form>

    And here is how my delete looks like now:

    <?php
    
    require_once("../../../wp-admin/admin.php");
    global $wpdb;
    
    $table_name = $wpdb->prefix."comment_reminder";
    $blogid=$_GET['id'];
    
    if ( !empty($_POST) && check_admin_referer('verify_creminder','creminder_nonce') )
    {
       $wpdb->query( $wpdb->query( "DELETE FROM $table_name WHERE id='$blogid'" ) );
    }
    
    header("location:/wp-admin/options-general.php?page=comment-reminder");
    
    ?>

    Now the only problem is - i doesen't delete the URL, as it should.
    I get no errors what so ever - so I really don't know what's wrong here. Now I don't know if it has anything to do with my delete LINK - here it is:

    foreach ($cremindsql as $cremind)
    	{
    		echo '<div class="cr_bloginfo"><strong><a href="/wp-content/plugins/comment-reminder/comment-reminder-delete.php?action=del&id='.$cremind->id .'">Delete</a></strong> - <a href="'.$cremind->blogurl .'" target="_blank">'.$cremind->blogname .'</a><br /></div>';
    	}
    }

    I hope I'm on the right path.

    Thanks
    Aris

  13. bcworkz
    Member
    Posted 1 year ago #

    Aris, you're on the right path, but you've strayed off into the weeds.

    You seem to be confused on what data gets sent to your server when and how. There's other methods, but for our purposes with WP, we can say all data is sent by browsers by one of two types of requests: GET and POST.

    Links are always GET requests, the only data sent must be in the URL itself, other than some header data, nothing else about the page is sent. In particular, the form data is not sent. The PHP page receiving the request can get parameters in the URL as parts of the $_GET array.

    Form data is sent by what ever method is specified by the method attribute in the form tag. The data sent is the names and values of all form elements within the form tags, nothing else. If the method is GET, each form element's name becomes an url parameter that is equal to the element's value. Forms with many fields result in very long URLs so this is often not a good method for forms.

    Forms are better sent with the POST method, in which case the data is sent in a separate data packet than the URL and is retrieved in PHP as the $_POST array. Again, only the names and values of elements between form tags is sent, and there are no URL parameters at all.

    So now you should see that the nonce in your form will never be seen when the delete link GET request is sent. Now that you have a nonce check in place, it will always fail. You need to build your delete link with wp_nonce_url(). Then the nonce check will actually have something to check.

    But don't get rid of the form nonce! Just as much as you should ensure the delete request is from a proper source, you should ensure the insertion of data request is from a proper source as well. But use a different nonce name to avoid confusion. (I know,.. Arrrgh! More work!)

    I know all this security stuff is a lot of bother for just a simple delete request, especially when you're learning. Unfortunately, by installing WordPress, you've made your site a target for hack attempts. Fortunately, the core WordPress is very secure, but you need to do your part to maintain it when you extend it's capabilities. Especially if others will be using your code. If you don't, somebody will eventually find the hole. Once you get this all figured out the first time, writing secure code will become second nature.

    Keep up your worthy efforts. I hope you are finding some enjoyment from this.
    Happy coding,
    bc

  14. Kuckovic
    Member
    Posted 1 year ago #

    Hi bcworkz!

    Good news!
    I've managed to create a nonce, and verify it!
    Or at least I guess I did....

    Heres my "Delete link":

    global $wpdb;
    $table_name = $wpdb->prefix."comment_reminder";
    $cremindsql = $wpdb->get_results("SELECT id, blogname, blogurl FROM $table_name ORDER BY id DESC");
    $nonce= wp_create_nonce  ('my-nonce');
    	foreach ($cremindsql as $cremind)
    	{
    		echo '<div class="cr_bloginfo"><strong><a href="/wp-content/plugins/comment-reminder/comment-reminder-delete.php?action=del&id='.$cremind->id .'_wpnonce='.$nonce .'">Delete</a></strong> - <a href="'.$cremind->blogurl .'" target="_blank">'.$cremind->blogname .'</a><br /></div>';
    	}
    }

    And my delete.php:

    <?php
    
    require_once("../../../wp-admin/admin.php");
    global $wpdb;
    
    $table_name = $wpdb->prefix."comment_reminder";
    $blogid=$_GET['id'];
    $nonce=$_REQUEST['_wpnonce'];
    if (! wp_verify_nonce($nonce, 'my-nonce') ) die('Security check'); 
    
    $wpdb->query( $wpdb->query( "DELETE FROM $table_name WHERE id='$blogid'" ) );
    
    header("location:/wp-admin/options-general.php?page=comment-reminder");
    
    ?>

    At least now I get the "Security Check" ....
    Thats progress for me.. :P

  15. Kuckovic
    Member
    Posted 1 year ago #

    Hi bcworkz!

    I figured it out - all by myself.. :P
    I looked into my link, and found out, there was a "&" missing between the ID and the "_wpnonce" - Now, I can actually delete my record, and when I type in the link into the browser, and chance a number in the nonce, I get the "Security Check" error - that means it works!!!

    Now, I'm going to work a bit with the form-nonce :D
    When I get that thingie figured out, I should be able to pass the "WP Plugin security check" :)

    Am I right?
    or do I need to fix something more?

    - Aris

  16. bcworkz
    Member
    Posted 1 year ago #

    Good work Aris! You deserve a little celebration!

    The nonce exchange takes care of the bulk of security issues. You might need to check user capability, unless only users that see the form all have proper capabilities already. Doesn't hurt to check again though.

    Same goes for a sanity check. If the id comes hard coded from the form, there's little chance of it being damaging, but doesn't hurt to check anyway. Sanity checks really are more important for text fields where the user can enter anything. It's probably complete overkill to check machine generated ids. I mainly mention it for the sake of general completeness.

    I'm jazzed for you figuring this out. Cheers!
    -bc

  17. Kuckovic
    Member
    Posted 1 year ago #

    Hi bcworkz!

    Thanks!
    I celebrated it with a full cheese pizza!

    Now, I've read something about an admin_referer. It could check the nonce, but how? ... Every time I try to put in "check_admin_referer" - i get the error "Are you sure you want to do that" And can only press "Back" ...

    Sanitizing my input would be a good idea ..
    I could check the URL and Blogname textfield..
    Lets say "max 30 chars" and sanitize that ..

    Or do you have any other suggestions?
    Cheers

    Aris

  18. bcworkz
    Member
    Posted 1 year ago #

    check_admin_referer() checks that the referer field is from within the wp-admin folder structure. If your form is from the plugins folder, that is why it's failing. If your form is from wp-admin, then it's possibly just some silly syntax error.

    Limiting text length is good for limiting possible code injection, but no need to be too stingy on length. 30 chars for an url might be a little tight. Most importantly for urls is to reject characters that are illegal for urls, I'm not sure what they are though. Hard to restrict characters for arbitrary names though, but you might consider restricting characters that have special meaning in mySQL.

    Each field will have different requirements, you sort of have to think like a hacker to know how to sanitize input. Hard to do if you're not a hacker. Maybe research how SQL injection attacks are done?

    The learning never ends, eh?
    -bc

  19. Kuckovic
    Member
    Posted 1 year ago #

    Hi bcworkz!!

    I have, once again, "updated" my security on my plugin - now it checks if user can "manage options" before it calls the options page:

    function my_plugin_options() {
    	if ( !current_user_can( 'manage_options' ) )  {
    		wp_die( __( 'You do not have sufficient permissions to access this page.' ) );
    	}
    ?>

    Hurray!
    Now, I recieved a mail from the plugin reviewers, telling me following:

    Your options.php is calling wp-admin.php

    require_once("../../../wp-admin/admin.php");

    Really, you never need to include WP's core code like that. The whole point of the hooks and functions is so you don't :)

    Now, how can I do that???
    I've looked everywhere.... :(

    - Aris

  20. bcworkz
    Member
    Posted 1 year ago #

    Anytime a link requests a php file, it initiates a new process, which requires loading the WP environment. For example, examine the core files linked in the admin panel menu. Every one of them includes either wp-admin.php or wp-load.php, depending on the situation. If you do not do this, you cannot access any hooks and functions.

    I can only think of two ways around this, but both end up loading the environment in the background so you don't have to explicitly do it, but it still happens all the same. One is to link to an existing core file that accepts url parameters such as "action" which also provide hooks so you can handle custom actions. I can't offer up an example, but they probably exist. Then the core file requires wp-admin.php instead of you. I don't see the real difference.

    The other way you don't need to require wp-admin.php is to do an AJAX request. Again, the requiring of the wp-admin.php is done for you in the background by the AJAX post object. You simply hook the proper AJAX action. Once again, whether you load the environment or the AJAX post object does, it must happen, I don't see the real difference.

    Now if you were to simply include files unequivocally, I could see the objection. But require_once() sees to it the files are only loaded because they have not been loaded yet. If they are not loaded, there are no functions to call and no filters and actions to hook.

    I hope this helps you understand this issue.
    - bc

    BTW, if you decide to continue requiring wp-admin.php for your plugin which would be distributed, you should get the proper path to it through some function instead of the relative ../../../ just in case someone has some funky folder structure. See the core files linked in the admin panel for some examples.

  21. bcworkz
    Member
    Posted 1 year ago #

    Another post reminded me of a third way to not include wp-admin.php, the form could submit to it's own page, which is what happens when the action parameter is an empty string. But what happens in the background is the WP index.php is called to find the correct page, which includes the wp-load.php page as one of it's first tasks. So once again, it's done for you in the background, but it's done all the same, so I don't see the real difference.

  22. Kuckovic
    Member
    Posted 1 year ago #

    Hi bcworkz!

    As you say, whether I include the core-file, or a function includes it, it really doesen't make any difference?...

    Well well...
    I've got my files, and my structure is like this:

    pluginname.php (setting up connection to database etc.)
    pluginname-admin.php (Creates a "optionspage" in the admin-section)
    pluginname-dashboard.php (Creates a dashboardwidget, and outputs some results)
    pluginname-delete.php (Handles the delete-link from "pluginname-admin.php")
    options.php (Handles the forminput from "pluginname-admin.php")

    Now, the files that requires "admin.php" are:

    - pluginname-delete.php
    - options.php

    The other files does NOT include it.
    And my "pluginname.php" are handling some MySQL queries aswell.

    But if I delete the "require_once" line in the files - let's say in the delete-file, it outputs an error, due to "wp_verify_nonce" ...

    What should I do?
    I really can't think of anything anymore.. :(

    If you can, please give me some examples.

    - Aris

  23. bcworkz
    Member
    Posted 1 year ago #

    Aris,

    Those other files have hooks and functions to extend WP and are not called directly by the client, which is the model your reviewers are thinking of. When called directly by the client, something has to load the WP environment or WP will not work.

    Honestly, you should politely thank your reviewers for their input but you are going to continue to use the current method of loading the WP environment. Because your data is separate from WP, but you need to use WP functions, and there are no filters or actions that meet your needs, you have no alternative but to load the environment this way. You've followed the good example set by WP core developers for handling settings forms submits. For example, the admin settings forms submit POST requests to wp-admin/options.php. The very first code line of that file is require_once('./admin.php'). You are comfortable following their good example.

    I hate telling people that are trying to help me they essentially don't know what they're talking about, but it's sometimes necessary. If it's really important to appease them (Perhaps they're paying for the project?), despite their misunderstanding of the concept, there is an alternative. You could use AJAX to handle the submits and deletes. All ajax requests go to admin-ajax.php, so your script files have no need to load the WP environment. (you can easily guess what one of the first things admin-ajax does)

    Unfortunately, the Codex documentation for AJAX is weak. There's adequate information on the 'net, but it's confusing to put all the parts together properly unless you have pretty good skills in both jQuery and PHP. Us mere mortals can still work it out, but it's a struggle. It is a very important aspect to learn though, so worth the effort.

    If (when?) you go down this path, I suggest you avoid the example in the Codex that uses inline code and figure out how to enqueue and localize external javascript files, it's an overall superior approach IMO.

    Good luck with whichever path you choose.
    -bc

  24. Samuel Wood (Otto)
    Tech Ninja
    Posted 9 months ago #

    Honestly, you should politely thank your reviewers for their input but you are going to continue to use the current method of loading the WP environment. Because your data is separate from WP, but you need to use WP functions, and there are no filters or actions that meet your needs, you have no alternative but to load the environment this way.

    This is incorrect. There is never a correct case where a *plugin* should be doing any form of include with a "../.." sort of thing to load WordPress files.

    Specifically, WordPress supports redefining the location of the wp-content folder, so you cannot be sure that ../../../whatever refers back to the main folder of the WordPress installation. Having the wp-content folder completely outside the WordPress core install is a supported case, and plugins need to support that case properly.

    We do not allow plugins that do this sort of thing in the directory any longer. Some older plugins may still be doing it wrong, but one step at a time.

    If you need to make direct HTTP calls to your plugin to get data, then using the admin-ajax.php file and the various AJAX hooks (as explained in AJAX in Plugins) will give you the ability to have WordPress load, load your plugin, and then let your plugin produce output. This gives a constant and correct URL by which you can access the system and have it load your plugin to produce whatever output you like (not necessarily just javascript oriented return data).

    If you're doing some kind of admin settings screen as this thread seems to be describing, then using the Settings API or simply registering your screen to make calls back to options.php is a better case. You don't need PHP files that you call directly to perform WordPress related actions. WordPress loads plugins, not the other way around.

  25. bcworkz
    Member
    Posted 9 months ago #

    Otto, thank you for taking the time to respond. The comments I made back then were obviously naive, I've since educated myself on this issue and hopefully have been giving out better advice more recently.

    I'm still not fully clear on what the real issue is, though I've identified alternative approaches such as AJAX. I completely agree ../../ path references have no place in the WordPress environment. But what if one were to include wp-load.php using site_url()? It resolves the relocated wp-content issue, but it's still conceptually poor as far as "WordPress loads plugins, not the other way around."

    As you noted, alternatives are available in the OP's case. But generally speaking, people are developing plugins that really are unrelated to WordPress core but need some minimal WordPress resources, say some user data. Sure, the data can be retrieved from the DB directly without involving WordPress at all. But we are seeing development from folks who are not the strongest PHP coders. It's all too easy for them to include wp-load.php and call wp_get_current_user(). Would that be so bad, assuming the ../../ structures are avoided?

  26. Samuel Wood (Otto)
    Tech Ninja
    Posted 9 months ago #

    The site_url() function gives you a URL. It doesn't make any sense to include a URL.

    And yes, including wp-load.php is always a bad idea, because, again, you have no valid way to retrieve the directory path (not the URL path) of the location of the wp-load.php file in order to include it.

    If you need to make an HTTP request to your plugin from the browser, then you should be making an HTTP request to WordPress, and passing parameters along that your plugin can recognize, and then having the plugin take appropriate action based on that. Considering that this is precisely what the admin-ajax.php method does, that's probably the way to go.

  27. bcworkz
    Member
    Posted 9 months ago #

    Yeah, site_url() was a bad example, I see what you're saying now. I had my head on backwards and am sufficiently embarrassed. Be assured I'll no longer advocate such a cheap shortcut. I appreciate your feedback. Thank you.

  28. bcworkz
    Member
    Posted 7 months ago #

    I have more information to share regarding the problem of accessing WordPress functions from external files. Otto sagely suggests using AJAX. Such a solution is almost always possible, though not always desirable. As it happens, there is a similar technique to the AJAX approach, except no javascript or jQuery is required, your request handler can be initiated from a simple HTML link or form action. It is suitable for any GET or POST request. The handler can include another PHP file and in the process enable access to WP functions on that file.

    See Plugin_API/Action_Reference/admin_post_(action).

    Better late than never :)

Topic Closed

This topic has been closed to new replies.

About this Topic