[theme-reviewers] Direct access prevention in comments.php - required or recommended?

[Justin Tadlock]

Basically, the code is ultimately irrelevant.  There's no security issue 
though.

It's good to point it out to theme devs that they don't need it.

On 9/23/2011 9:52 PM, Dion Hulse (dd32) wrote:
> The use-case mentioned is completely different from what Mark's post 
> is about. the snippet mentioned prevents direct access to the comments 
> template, Marks post is about using $_SERVER vars in theme output.. so 
> you can ignore that posting
>
> Speaking from a technical point of view (and not one of knowing what 
> the exact theme review guidelines are):
> Having that in there is pointless, but having it in there is not a 
> problem either. It's just extra code being run, it's never going to 
> prevent a problem, nor create one (Well, unless the theme is doing 
> strange things in the comments template - like writing files or 
> something - things that that file should never do).
> So the guidelines are suggesting removal of a useless piece of code. 
> No need to require it's removal, it's something like doing this: if ( 
> false ) {run_some_code();} - it's code thats never going to be of 
> use.. Only thing removing it does it make people more aware of what 
> the PHP is actually doing..
>
> As for using $pagenow/wp_die(), it's not needed, nor would it be 
> available in that case either (direct access of the file). If the 
> comments template is included, the theme is asking/WordPress is asking 
> to output the comments... bit of a no-brainer that it wouldn't need to 
> check the current templating action.
>
>
> On 24 September 2011 11:37, Chip Bennett <chip at chipbennett.net 
> <mailto:chip at chipbennett.net>> wrote:
>
>     I can't say that I agree that it's a security risk; it's a
>     conditional, not an input/output.
>
>     However, I'm not sure it's really *needed*. What is the inherent
>     risk of loading comments.php directly?
>
>     If it *is* needed, what about using $pagenow instead (I assume
>     it's available in the front-end)? e.g.:
>
>         global $pagenow;
>         if ( 'comments.php' = $pagenow ) {}
>
>
>     Also, might it be worthwhile to use wp_die() instead of die()?
>
>     Chip
>
>
>     On Fri, Sep 23, 2011 at 8:24 PM, Tyler Cunningham
>     <seizedpropaganda at gmail.com <mailto:seizedpropaganda at gmail.com>>
>     wrote:
>
>         You are correct in requiring this. It is actually now a
>         security risk as pointed out by Mark Jaquith in a blog post.
>         You can link to this post if you like:
>
>         http://markjaquith.wordpress.com/2009/09/21/php-server-vars-not-safe-in-forms-or-links/
>
>         Regards,
>
>         Tyler Cunningham  |  Founder, COO - CyberChimps LLC
>         <http://CyberChimps.com/>
>
>         @tylerbcunning <http://twitter.com/tylerbcunning>
>         http://gplus.to/tylercunningham
>         http://linkedin.com/in/tylerbcunningham
>         tyler at cyberchimps.com <mailto:tyler at cyberchimps.com>
>
>         On Friday, September 23, 2011 at 6:23 PM, Vicky Arulsingam wrote:
>
>>         I'm seeking clarification regarding the use of:
>>
>>         if ( 'comments.php' == basename($_SERVER['SCRIPT_FILENAME']) )
>>         die ( 'Please do not load this page directly. Thanks.' );
>>
>>         I've been requiring that themes not include this. Am I
>>         correct in doing so or is the removal merely a recommendation?
>>
>>         -----
>>         Vicky Arulsingam
>>
>>         _______________________________________________
>>         theme-reviewers mailing list
>>         theme-reviewers at lists.wordpress.org
>>         <mailto:theme-reviewers at lists.wordpress.org>
>>         http://lists.wordpress.org/mailman/listinfo/theme-reviewers
>
>
>         _______________________________________________
>         theme-reviewers mailing list
>         theme-reviewers at lists.wordpress.org
>         <mailto:theme-reviewers at lists.wordpress.org>
>         http://lists.wordpress.org/mailman/listinfo/theme-reviewers
>
>
>
>     _______________________________________________
>     theme-reviewers mailing list
>     theme-reviewers at lists.wordpress.org
>     <mailto:theme-reviewers at lists.wordpress.org>
>     http://lists.wordpress.org/mailman/listinfo/theme-reviewers
>
>
>
> _______________________________________________
> theme-reviewers mailing list
> theme-reviewers at lists.wordpress.org
> http://lists.wordpress.org/mailman/listinfo/theme-reviewers
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.wordpress.org/pipermail/theme-reviewers/attachments/20110924/e374fafd/attachment.htm>