hesseldj
commented
1 year ago Hi @HubertGL,
Thank you for post. Don't worry about the amount of text. My answer will be quite long as well :)
Why are we not requiring a file with a dynamically? I think you are right that we could use WP_CONTENT_DIR. The reason why we didn't do that is, because at first we wanted to not load Wordpress at all. But quickly we found out that we need to use it for accessing the database securely. So we also added wp-load.php and I did not think about changing the hardcoded path to a dynamic path. The path is dynamically created in the plugin though(class-endpoint.php). So it should not cause any issues, but I think making the path dynamic is always better.
So I think we should follow your advice and use WP_CONTENT_DIR. Thank you for bringing this up to us.
Why are we putting burst-statistics-endpoint.php in the root? We thought that it would be a good place because of the access and write permissions in the root. But after discussing this with @rlankhorst, we came to the conclusion that the /wp-content folder might be a more logical option. Because that is where custom changes to the Wordpress should be placed.
Regarding security of the root folder, the root folder should be writable for Wordpress updates to work. So I don't think that is really an issue. Or is there a flaw in my thinking?
Will the content of burst-statistics-endpoint.php always be the same per version? Yes, it will not change based on settings. Only the hard coded directory will change depending on how wordpress is installed. But as I said we should follow your recommendations and change it to a dynamic path.
Thank you for valuable insights. We will think about moving the endpoint to a more suitable location and we will definitely change the hard coded path in the endpoint to a dynamic path. Please let us know if you have more ideas. It really helps us improve our product.
Kind regards, Hessel
Hi, this might maybe not an issue to anyone but to some it might. I guess the burst-statistics-endpoint.php is generated by your plugin and at least 2 out of 3 include paths are fully hardcoded, which can make trouble when you have different root folders. That's how it looks in my case:
define( 'SHORTINIT', true ); require_once DIR . '/wp-load.php'; require_once '/var/www/html/wp-content/plugins/burst-statistics/helpers/php-user-agent/UserAgentParser.php'; require_once '/var/www/html/wp-content/plugins/burst-statistics/tracking/tracking.php'; burst_beacon_track_hit();
Since the file already included wp-load there should be no issue to include via wp constants like: require_once WP_CONTENT_DIR . '/plugins/burst-statistics/helpers/php-user-agent/UserAgentParser.php'; require_once WP_CONTENT_DIR . '/plugins/burst-statistics/tracking/tracking.php';
For better understanding, is there a valid reason to generate the burst-statistics-endpoint.php within the root folder instead of using the standard wordpress way? Because that creates actually security flaws for the app, since the webserver should not be allowed to write executables especially within the root folder. The more important question: will the content of burst-statistics-endpoint.php always the same within the same plugin version of course?
Sorry for so much text and thanks for your plugin