[theme-reviewers] [wp-testers] A RE-EXAMINATION OF THEME REQUIREMENTS - AN ESSAY

[Chip Bennett]

Moving this discussion back to the appropriate list. Responses are inline.

On Mon, Jun 27, 2011 at 5:13 PM, Bruce Wampler <brucewampler at gmail.com>wrote:
>
>
> 1. EVER CHANGING REQUIREMENTS - THERE MUST BE SOME GRANDFATHER PROVISIONS
>

The claim that the Theme Review Guidelines are "ever-changing" is
demonstrably false. As I have stated already: we have implemented a policy
of making changes to the Guidelines in sync with the release of major
WordPress versions. There may be exceptions (if we discover a security
exploit or other similarly egregious issue, we'e not going to wait to
implement), and we're certainly not human. But we have stuck to this policy
for the past two major-version WordPress releases.

>
> My theme has been in the repository for over a year now, and during that
> time I have spent countless
> hours revising it to meet various new requirements for theme approval. I
> have found the sheer
> number of changing requirements excessive, and arbitrary. But perhaps the
> worst part is that
> all the requirements are applied to each theme submission - whether the
> theme is new or old.
> I have literally spent days bringing my theme into compliance, only to have
> it fail the next time,
> a few weeks or months later, because of some new requirement that may or
> may not make the theme better.


Your experience is understandable, but it is atypical. Your Theme is
considerably more complex that most, and introduces far more potential
issues. Also, you first submitted the Theme back when the Theme Review Team
had only just been formed, and the Guidelines were still very much in flux.
Things changed - a lot, and often - a year ago, but they are not nearly as
dynamic today.

>
> I accept that meeting security measures is required - except when it is not
> really clear that those
> measures bring about true security. I find some of the measures the
> equivalent of taking my shoes
> off for the TSA - no one in the end is any safer, they've just wasted a lot
> of time.
>

If you have a concern or disagreement about a specific security requirement,
by all means: bring it up. We try to provide ample means to discuss these
issues. The Team have prompted many discussions specifically regarding Theme
security, both on the mail-list, and on the Make site. Please avail yourself
of the opportunity to participate in those discussions, or feel free to
start your own. But I reiterate: please discuss *specific* issues. Blanket
complaints aren't helpful, because they don't help us address specific
issues.

>
> But what is the most difficult is meeting arbitrary requirements that are
> really a matter of style,
> or a simple way to make reviewer's lives easier. (And I appreciate the
> difficulty of trying to
> improve those standards.)
>

Arbitrary? Hardly. Again: please be specific here. What do you consider to
be arbitrary?

>
> I'd like to use one example from my own theme to explain the problem.
> Earlier this year, I was
> convinced by Chip that I needed to switch to the settings API for my
> options pages. My theme is
> somewhat different than most in that it has lots of options - like
> hundreds. (And essentially every
> single one of options was added in response to multiple feature requests
> from my user base.)
>
> So I did in fact spend about 100 hours converting to the Settings API. (I
> had been using
> forms with nonces before.) But I had a big problem - there was (and may
> still be) a significant
> bug in the Settings API. If the same data base item was manipulated with
> two instances of forms
> using the Settings API, one case would wipe out the sub-settings
> manipulated by the other instance.
> I spent countless hours tracking this down, and in the end, decided it was
> likely a database cache
> problem. The result, however, was that I needed to used two database
> entries - one for each of
> the different Settings API forms. This also involved substantial
> reorganization of my
> internal structure to accommodate the two entries.
>
> At the time I did all this (and I was working with Chip all along to make
> the theme meet
> submission specifications as well as using the Settings API), there was no
> requirement to
> have only a single settings entry in the database, but that is now a
> requirement.
>

No; it was a requirement *then*, and it remains a requirement *now*. I told
you as much at the time - and I also told you that the Team agreed that,
given the sheer volume of options in Weaver, that splitting them into two
options arrays was a valid exception to the single-database-options-array
requirement. So, nothing has changed with the requirements, and you would
still be granted that same exception today.

>
> So what do I do? I spent a long time working with one of the lead reviewers
> to use the Settings
> API, which they consider important, and I think are under consideration to
> make required.
> Because of limitations of the Settings API, I was forced to use two
> database entries, and
> that was OK in March. Now, in June, that no longer is OK.
>

It won't be required any time soon, though perhaps, far enough into the
future. As has been mentioned: the Settings API still needs some maturation
with respect to Themes. We strongly recommend it, because, for
less-experienced developers, it is the easiest way to ensure a LOT of the
data-security issues are satisfied. But Themes that choose to handle those
issues on their own are still accepted - provided that their custom
implementation does, in fact, satisfy all security concerns.

>
> It seems clear to me that the review process must allow for exceptions,
> especially for
> themes that have been previously approved. In this case, allowing more than
> one
> database settings entry seems harmless. If the argument is that having more
> than one
> entry will break future uninstall plans, I would say than that the
> uninstall plans
> should be changed to allow that. I can think of several good reasons to use
> several
> database entries for theme settings (certainly not unlimited, but say three
> or four).
>

You WERE granted an exception regarding number of database options. So, it
would seem that this point isn't really an issue.

>
> This is but one example. I'm sure there are many other requirements that
> might
> improve overall theme quality, but that may be very difficult to meet for
> some
> existing themes because of internal architecture. (For example, table based
> themes
> vs. CSS block based themes - that might reasonably be required for new
> themes,
> but it would be unreasonable to block updates to existing themes because
> they
> are table based.)
>

As far as I know, no Theme has been not-approved due to a table-based
layout; of course, I don't know that I've reviewed any such Themes. But,
there is nothing in the guidelines that explicitly forbids table-based
layouts, as long as CSS is properly abstracted from the markup.

>
> So I would like to propose that existing themes be given some grandfather
> protection
> from having to go though major internal restructuring to meet specific
> guidelines. This
> may require a special submission process, or an option for the automated
> submission
> interface.
>

I don't see this happening. WordPress continues to evolve, and introduce new
features and functionality. APIs change. New exploit vectors emerge.
Implementing Guidelines changes in sync with the WordPress major-version
release cycle facilitates incorporating version-based WordPress changes into
the Theme review process, and I see no reason to "grandfather" Themes in
this regard.

I believe that WPORG repository-hosted Themes carry an implied requirement
to be kept updated, and to be kept current with the underlying WordPress
core codebase. As Matt M. has stated: the goal of the repository is not to
host *every* Theme, but to host the *best* Themes. Building in
grandfathering does not further that goal.

>
> To not make an official upgrade/grandfather policy, I believe, will
> ultimately be
> harmful to the WordPress community.


I disagree. To allow Themes to remain in the Repository without being kept
current harms the WordPress user community. To "grandfather" Themes, and
allow them to update without incorporating current core features and
functionality harms the WordPress user community.


> Personally, I have a long record of contributing
> open source software. I want to keep my theme alive and up to date on
> WordPress.org.
> But I will not re-write the interface again to meet an arbitrary
> requirement for only
> one database entry.


Has anyone asked you to do that? Can you point to the ticket where you were
asked to do that? Point me to it, and I'll explain the exception that has
been granted to your Theme, so that the issue can be put to rest.


> I will simply withdraw from WordPress.org, and try other options.
> I would guess that other great theme will also be removed or die of neglect
> because
> it is simply too hard for programmers who are giving away their programming
> efforts
> to update existing themes to meet new and ever-changing requirements.
>

That is your choice - though speaking personally, it is one that I hope you
choose not to make.

>
> 2. ATTACK SECURITY CORRECTLY AND COMPREHENSIVELY
>
> I find the approach to security used by the WordPress theme review process
> less than
> optimal. The philosophy seems to be a that it all must be done in the
> theme, even to
> the extent that themes can't use tools available to core WordPress. At
> the same time, there are no controls whatsoever for plugins. I guess one
> could
> argue that everyone needs a theme, and plugins are options. But that
> doesn't take
> into account reality. It would be a real challenge to find a WP site
> without a
> plugin.
>

If you have issues with how Plugins are handled, please bring it up with the
appropriate people who deal with Plugins. But appealing to the handling of
Plugins will never be a valid argument when discussing Themes. Just because
there are deficiencies with Plugins does not mean that those same
deficiencies should be allowed to remain with Themes. Further: they're just
simply different beasts. The approval process is different. The SVN handling
is different. The way they are viewed and used by end users is different.

But if you believe that the Theme Review Team settings review process and
philosophy sub-optimal, then (again): address specific areas for
improvement.

>
> For my example, I'd like to discuss the ban on fopen. I've read the reasons
> and arguments
> for the ban, but I remain unconvinced of the necessity. To summarize, the
> argument
> is that on shared hosts, using fopen to create a file will result in a file
> ownership of Apache or nobody, which could then be exploited by other
> accounts on
> the same shared server.
>
> The problem is - just how many commercial shared servers exist that
> actually work
> that way? Seriously. I only have access to two shared servers, but they
> both
> always set all ownership to the account holder automatically. It is
> impossible
> to create a file using fopen that doesn't belong to the account owner.
>

For discussion of specific issues of file operations and permissions, again
I defer to Frumph and Otto, et al, because it's not my area of expertise.

>
> The arguments about ownership don't apply to standalone servers such as VPS
> or others.
>

In such matters, the Guidelines must address the *lowest* common denominator
- which would be the ubiquitous, low-cost, shared-hosting environment,
rather than VPS or dedicated servers.

>
> So there probably are some shared sites that don't work that way, and
> create
> accounts with apache/nobody as owners. From my perspective, forcing themes
> to use the unfriendly WPFilesystem instead of fopen is the wrong solution
> for several reasons.
>
> 1. It is unfriendly. Some users will be forced to enter their FTP
> credentials
> over and over, or modify the wp_config.php file to include the FTP info. Or
> keep the info in the database - oh wait: only one database entry is
> allowed.
>

If I'm not mistaken, such users *already* have to enter FTP credentials, for
performing core/Plugin/Theme updates using the automatic updater.

>
> 2. It totally ignores the possibility of the unrestricted plugins that can
> use fopen as freely as they like.
>

Irrelevant. Can we stop bringing up what Plugins can do, and how they
behave? It's just simply not going to be an entertained argument, because we
have absolutely NO control over Plugins.

>
> 3. The current implementation of the WordPress media library does not
> ensure
> that media files are owned by the site owner - they are created and owned
> by apache or nobody on sites configured that way.
>
> An how, in any way, shape, or form, does a ban of fopen to read files have
> anything to do with security at all? The only thing I can think of is that
> Theme Check is too lazy to distinguish the difference.
>

Theme Check is open source. If you can improve the codebase, Pross and Otto
would welcome with open arms any patches you wish to contribute.
#patcheswelcome

>
> And I think this is the wrong approach. It is way too micro-managed. A much
> better
> approach would be to make the solution a bit more global - one that would
> help protect sites that use plugins as well as themes. One that would
> educate
> the end users, and help web security as a whole.
>

A laudable goal, but one that is outside the scope of the Theme Review Team.
We are tasked with reviewing and approving Themes submitted for inclusion in
the WPORG repository.

>
> How to raise the level higher? Put some checking functionality into
> WordPress.
> Have WordPress run a basic security check.
>

Again: #patcheswelcome. Until then, the Theme Review Team must operate on
the playing field as it currently exists.

>
> Take fopen - from my small sample, I would conclude that an increasing
> number
> of shared hosts don't have a file ownership problem that could be exploited
> by other accounts on the same server. The goal should be to make that
> number
> zero. You don't accomplish this by micro steps way down in themes. You do
> it by providing feedback to the end user:
>
> WARNING! WordPress has detected a potential exploit path on your site's
> host or
> server. If you are using a shared hosting company, you should consider
> changing hosts to a different company. If you are not running on a
> shared host, you can ignore this message.
>
> Provide appropriate wording - links to explanations, etc. But the goal is
> to shutdown badly configured shared servers, or encourage them to configure
> their sites to make all file ownership go to the owner and not the server.
>
> Now we have a solution that helps everyone. Plugins that use fopen are now
> safer. Theme writers can use methods that lead to an optimal user
> experience.
> And we don't waste anybody's time on servers that aren't affected by a
> problem that should handled at that level anyway.
>

Again: a laudable suggestion - but one that is entirely outside the scope of
the Theme Review Team. Bring it up on the Hackers list. Open some core Trac
tickets, and submit some patches. Start the discussion at the *core* level.

>
> (On the other hand, I believe that themes should flash big warning messages
> to people still using IE6, and even IE7, for that matter. We'd more likely
> prevent far more evil things that way than banning fopen from themes.)
>
> IN SUMMARY
>
> 1. Grandfather previously accepted themes
>    It is not fair to force authors of previously accepted themes to go
>    through major re-writes whenever they submit a new version of their
> theme,
>    or a bug fix.
>

No. It is entirely fair. You seem to believe that the Guidelines should be
set in stone, forever and ever, Amen. That's simply not going to happen. We
will continue to strive not to be arbitrary or capricious with changes to
the Guidelines, but they WILL continually improve, and WILL track with
changes to WordPress core.

It would be unfair to *end users* to allow Themes to be grandfathered, which
would provide the opposite incentive for the developers of the most popular
Themes NOT to update their Themes, to the detriment of end users. (Don't
believe me? Spend some time in the WPORG forums, and see for yourself just
how many of the issues in the Themes and Templates forum are a direct result
of users using outdated Themes.)

>
> 2. Get real with the security
>    Security is important. Placing all the burden on theme writers for
> issues
>    that can and should be addressed at other levels is also unfair.
>

Aside from fopen(), what specific issues do you have with the security
guidelines?

Also, the very reason that we strongly recommend using the Settings API is
so that developers don't *have* to take on the vast majority of the burden
of dealing with security issues, because implementing the Settings API does
most of the heavy lifting.

>
> Thanks for listening. I really want the WordPress community to have access
> to
> the best themes possible, but I believe some of the theme requirements have
> gotten out of hand over the past year, and lend nothing to the
> functionality
> and security of sites. There seems little appreciation for the efforts made
> by theme writers, or any mechanism for handling special cases.
>

"...some of the Theme requirements": again, be specific, or we can't hope to
change anything.

And again: you yourself were granted an exception, regarding the use of a
single database options array; so you are living proof that the Theme Review
Team does respond on a case-by-case basis.

>
> I really want to keep my theme on WordPress.org, but I'm really not sure I
> want
> to waste more and more hours meeting ever changing, and arguably overly
> restrictive
> and irrelevant theme requirements. It is good to have standards, but as
> someone
> who has been writing themes for 4 years, I find the constant moving target
> pretty difficult to tolerate, especially when the changing requirements
> will
> force fundamental redesign of a theme which was initially designed to
> meet the requirements at the time the theme was created.
>

And for the last time, I need to lay to rest the "ever-changing
requirements" canard. Here's the revision history of the Theme Review
Guidelines Codex
page<http://codex.wordpress.org/index.php?title=Theme_Review&action=history>.
Almost nothing changed from when you last submitted your Theme until now.
What was a requirement then remains a requirement now. Yes, the uploader
script didn't reject Theme submissions for WARNING/REQUIRED issues at the
time - but that was just because we hadn't yet thrown the switch. The actual
Guidelines - what was actually a warning or required - didn't change from
before then until now. It's simply enforced earlier.

(And for the record: you were ALSO granted an exception for the fopen()
issue. It was a requirement then, and I personally advocated to let it slide
for the time being, based on the effort you had made to incorporate the
Settings API.)

>
> If WordPress.org want to continue to attract top notch free themes, I think
> something has to change. I haven't decided yet, but I'm really close to
> abandoning the free theme world, and going down the premium path. It would
> be a shame.
>
> From what I've seen of a LOT of commercial Themes: the latest Themes in the
WPORG repository are FAR superior, in terms of code quality, security, and
support for core WordPress features and functionality. I think more and more
people are starting to realize that. You could choose to go the commercial
route (again, I personally hope that you don't), but I don't think it would
be as fruitful as you might imagine.

Chip
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.wordpress.org/pipermail/theme-reviewers/attachments/20110627/d409b8be/attachment-0001.htm>