Support » Plugin: WordPress Infinite Scroll - Ajax Load More » Good but some tweaks needed

  • This plugin doesn’t behave well in certain hosting environments, like Pantheon due the lack of write access.

    The plugin should work with shortcodes instead of php code, storing php code in the database that later is replicated into files appears to be a bad idea in terms of security.

    You shouldn’t invoke a die method in your update function when an error occurs, you break the admin, forcing to delete this plugin or disable it manually in the database.

    ****** UPDATE *******

    1st paragraph deleted, as I sounded like an a**hole speaking.

    As every software, sometimes you have a different point of view of how things should behave, and I say this is a good plugin but I think that it still needs to address the issues I previously mentionated, I’m changing my original review from 2 stars to 4 as the plugin works well and only needs to considerate some minor tweaks.

Viewing 5 replies - 1 through 5 (of 5 total)
  • Plugin Author dcooney

    (@dcooney)

    Hi Javier,
    I appreciate your review even though is not favorable to my plugin.

    I don’t ask for write permissions over the entire plugin folder, just the ALM templates directory – Do you have a suggestion for allowing users to create and save their own template files without having read/write permissions to directories on their own server?

    I understand some users do not have access to read/write and I am currently developing a system that will allow them select a file from the server rather than write to the server.

    It’s also my understanding the php shorttags are not available in all hosting environments.

    I will look into removing the die methods if they are not needed.

    Thread Starter javiertoledos

    (@javiertoledos)

    I think I was a little bit angry when I first reviewed the plugin, it’s just that the die() calls really gets on my nerves, sorry for that.

    I think, if the plugin cannot write to the file or the plugin folder (like it’s our case with Pantheon environment, or environments where you version the code), it should use the database as fallback, I imagine you store the templates as files to improve load speed. Another option could be using memcache or redis if they’re available.

    Having the option to select a template file from the server would be great.

    When I mentioned shorttags, I meant short codes, like contact form 7 for example. Anyways, my only concerns were the die() calls that break the admin if the ALM templates folder is not writable (sorry, forgot to read the FAQ) and the fact that you let the users write PHP code, in terms of security, it would be better if the users are sandboxed somehow, like using shortags [title], [permalink],[time format=”F d, Y”], or using a templating language {{{ $post->title }}}. I know this would represent less flexibility.

    After all, I think it’s a good plugin with only those drawbacks, and your plugin deserve a better review than the one I originally gave.

    Plugin Author dcooney

    (@dcooney)

    No worries, I do understand your issues and appreciate your updated review šŸ™‚

    I can say that the reason Ajax Load More is required to write to actual files on the server is because that is the only way to be able to run the php functions in the repeater templates. By including these templates as include() files users can run the_content() and the_title() etc.

    This code is also written to the database for plugin updates. When the plugin is updated all templates are cleared and they need to created on update.

    Shortcodes – The plugin does work with shortcodes, in fact that is the only way to implement Ajax Load More. [ajax_load_more post=”post” posts_per_page=5″] etc…

    I am going to investigate the die() calls for my next release!
    Cheers,

    Thread Starter javiertoledos

    (@javiertoledos)

    That’s the thing I don’t think that the user should be able write php code directly in terms of security, despite of adding a lot of flexibility, someone who gains access to the backend would able to write arbitrary code to escalate privileges. The idea is to limit the user to use exclusively shortcodes (no more php) in the repeater templates to implement a kind of sandboxing. Anyways it’s just a suggestion, you always sacrifice something when improving security.

    I know I’m discouraging storing or letting the user write custom php code, but, if you’re going to execute the php code in the repeater templates and they cannot be written in files, you may be able to execute the repeater template code from database by doing an eval ( '?> ' . $repeater_template ); the eval function can run an arbitrary (and malicious) code but it’s what the plugin is doing already by including custom repeater templates from files.

    Maybe something like this

    $repeater_template_file =  alm_get_current_repeater ($repeater, $type);
    /* If file doesn't exists that means that the ALM template folder
     * is not writable, then use the database
     */
    if ( !file_exists ( $repeater_include_file) ):
      $repeater_template = alm_get_current_repeater_from_db ( $repeater, $type );
      eval ('?> ' . $repeater_template );
    else:
      include ( $repeater_template_file );
    endif;
    Plugin Author dcooney

    (@dcooney)

    I totally see your points here, but from my point of view if an Admin is worried about another user, whether it is a contributor or editor creating a damaging code they really have bigger issues than my plugin šŸ™‚

    I also purposely wrote the plugin to avoid using eval() and exec() functions. I’m not that was the best move but everything has worked out so far.

Viewing 5 replies - 1 through 5 (of 5 total)
  • The topic ‘Good but some tweaks needed’ is closed to new replies.