E_DEPRECATED and E_STRICT issues in SimplePie

[uuf6429]

I am running a sophisticated logging solution, which keeps a list of potential problems in my code (and whatever software I'm running).

This includes Wordpress. While it's nice of Wordpress to only cause a minimal amount of errors, SimplePie ain't so nice.

The file wp-includes/class-simplepie.php causes numerous issues mostly related to passing objects by reference (which is the default PHP behavior for some time).
There are also some two E_STRICT issues, one on line 9432 and another on 14950. These two are happen because the dynamic methods SimplePie_Misc::windows_1252_to_utf8() and SimplePie_Misc::get_element(), should not be called statically.

The fixes should be trivial in nature. I'm not expecting high priority, but it would be nice knowing that Wordpress dev team takes source code quality to heart.

Cheers,
Chris.

[nacin]

SimplePie is an external library, so your best bet would be to report these issues upstream: ​https://github.com/simplepie/simplepie/. We use their stable (and legacy) 1.2 branch, so I would not be surprised to find most of this fixed in the master 1.3-dev branch.

[uuf6429]

Replying to nacin:

SimplePie is an external library, so your best bet would be to report these issues upstream: ​https://github.com/simplepie/simplepie/. We use their stable (and legacy) 1.2 branch, so I would not be surprised to find most of this fixed in the master 1.3-dev branch.

---

Hello Andrew,

What is Wordpress' policy on problem with external libraries? Let's say I report these issues to SimplePie devs and they build a 1.2.1 to address my issues specifically, will that branch make it to Wordpress?

On a second thought, I could write a patch myself - the fixes are really trivial in nature.

Chris.

[ocean90]

#12367

[rmccue]

Chris, I'm the maintainer of SimplePie and I love hearing about bugs. :)

WordPress's current policy on SimplePie is to follow the one-dot-two branch. However, 1.2.x is intended for PHP 4 compatibility, so we have to do this.

They are, however, all fixed in 1.3-dev (the master branch on GitHub), so once 1.3 is released, they'll end up in WordPress.

nacin, I'm thinking that since we keep seeing these reports pop up that it might be an idea to note somewhere that the E_DEPRECATEDs/E_STRICTs are a known issue and explain it. Perhaps the codex somewhere?

[uuf6429]

Ryan, I don't want to be rude, but it's been 3 months.
We've had a decent list of Wordpress updates since then, and sadly no patch for SimplePie.

On the other hand, I don't think this is your problem either. The wordpress devs should make their decisions on what software to use better. In your case, even dedicate a couple of developers to get the code finished and ASAP.

I still have to keep removing the log warnings mentioned above, which span several pages. And I'm sure as hell I won't hardcode any bloat to filter out error messages just because there's a bad link in the development chain (again, I'm not pointing at you Ryan).

I'm sorry if I sound condescending, but that's the way it is. People out there can't do proper logging because of this issue, and others unaware of it have huge log files filled with irrelevant information. I don't think this reflects well on wordpress......perhaps the guys on stackoverflow/php do well to classify wordpress as bloat....

[rmccue]

Hi! Don't worry about being rude, I've been an open source developer for too long to take any comments personally. :)

So, I'd like to explain in detail the situation and why it exists. The relevant "errors" that are picked up are E_DEPRECATED and E_STRICT, which were both introduced in PHP 5 as a guide to developers on where they can improve their code. (I quote "errors", since they're not actually errors, but just notices that the code can be improved.) Given that they are simply guides, they are of no relevance whatsoever to users, which is why the recommended error_reporting setting for PHP does not include them.

The key to why they exist is that they were added in PHP 5. In PHP 5, a lot of class handling changed. The PHP developers wanted PHP 4 code to work without errors, but wanted to encourage developers to use the new styles of coding, so introduced these new "error" types.

The only problem is, SimplePie 1.2.x is the last branch to support PHP 4, so we can't take advantage of these new constructs. That poses the question though, why can't we make it work on both? In fact, we already do where we can. In PHP 5, clone was introduced to copy objects as opposed to using references. We use this in SP already if we can, but not all features can be implemented this way.

Looking at the individual errors:

• Pass-by-reference: in PHP 5, the ability to pass an object in by reference to a function/method was changed to be part of the function definition/prototype. In SimplePie, we really want to pass objects by reference, since they're huge in terms of memory. In order to achieve this for PHP 4, we have to pass it by reference in the function call, but this gives an error (E_DEPRECATED) on PHP 5.

• Dynamic methods called statically: in PHP 4, classes have methods, and that's about as detailed as it gets. In PHP 5, the concept of visibility was introduced, in addition to static methods/properties. SimplePie uses a few classes (namely SimplePie_Misc) statically as essentially namespaces. By all rights, these methods should be declared static. The only problem is, this facility does not exist in PHP 4, so we can't implement it for both. Hence, to maintain compatibility with PHP 4, we have to leave it.

Hopefully I've now established why SimplePie doesn't do it. The other part of the problem is, why hasn't SimplePie been changed? The answer to that is, it has. In the 1.3-dev version, we've completely thrown out all PHP 4 compatibility and SimplePie uses a whole bunch of 5-only stuff (autoloading being my favourite). However, 1.3-dev isn't just for discarding PHP 4 compatibility, so as per any project, other features and bugs have been fixed. This means that the code is considered unstable until 1.3.0 has been released, hence why WordPress is not using it.

So, that begs the final question: why haven't I released 1.3 yet? The answer to that one is simple: I haven't had time. Before I release it, I want to add more testing in (especially for regressions) and fix some more bugs in it. However, I'm studying a dual degree full time, in addition to attempting to launch a business while also doing freelance work. I also waste a lot of time (30 hours a week) on train rides where I can't do much except read/listen to podcasts. And on top of all that, SimplePie isn't the only library I develop.

We've been aware of the lack of time that every developer has for SimplePie for a long time. The two previous lead developers both had to back out due to time constraints, and for a time, we even had to announce the SimplePie project as being dead due to lack of developer time. We've asked popular projects which use SimplePie (WordPress, Drupal, DokuWiki, and many, many others) if they'd be able to contribute some resources to helping out, but unfortunately we never had much of a response. (Matt did donate hosting for us, which was a great help.)

None of this is any excuse of course, so I'm sorry for not being able to release SP 1.3.0. I don't like the way it is, but unfortunately I can't change it much at the moment.

Of course, if you'd like to help, there's a ​stack of issues a mile long for 1.3.0 and any help there would be greatly appreciated.

Thanks for your thoughts, and your time spent reading this. :)

[SergeyBiryukov]

Replying to rmccue:

However, 1.3-dev isn't just for discarding PHP 4 compatibility, so as per any project, other features and bugs have been fixed. This means that the code is considered unstable until 1.3.0 has been released, hence why WordPress is not using it.

Wouldn't it be possible to make an intermediate stable maintenance release just for PHP 5 compatibility, and then continue working on other features and bug fixes?

Replying to uuf6429:

I still have to keep removing the log warnings mentioned above, which span several pages.

In the meantime, you should be able to replace the SimplePie class with your fixed version via a plugin (the one from wp-includes/class-simplepie.php will not be loaded then).

[rmccue]

Replying to SergeyBiryukov:

Wouldn't it be possible to make an intermediate stable maintenance release just for PHP 5 compatibility, and then continue working on other features and bug fixes?

Absolutely, but there are other changes in there for PHP 5 compatibility, so that requires a full branch. Unfortunately, I don't have much time.

[dd32]

Replying to SergeyBiryukov:

Wouldn't it be possible to make an intermediate stable maintenance release just for PHP 5 compatibility, and then continue working on other features and bug fixes?

I think it's worth noting here, that the code in question doesn't technically have anything wrong with it, It works just fine as it is, and is 100% legit PHP. The only problem being is that it doesn't fit a PHP5 best practice, but that is something you have to accept when supporting PHP4 code (as it was designed to do in this case).

E_DEPRECATEDs/E_STRICTs are not something you should worry about logging when it isn't your code you're logging, If you're going to jump in and fix it up, then log away, but if you're simply going to request people fix it, consider changing your error reporting level to something less verbose.. After all, you're logging errors, not coding suggestions.

[rmccue]

Replying to dd32:

E_DEPRECATEDs/E_STRICTs are not something you should worry about logging when it isn't your code you're logging, If you're going to jump in and fix it up, then log away, but if you're simply going to request people fix it, consider changing your error reporting level to something less verbose.. After all, you're logging errors, not coding suggestions.

Exactly. The only person who these errors matter to is me.

[uuf6429]

Well, at this point, it's turning out to be a major nuisance. Might as well give fixing it a try. I should be happy I can afford some developer time which a major project (like Wordpress) couldn't.

dd32 - I disagree. When I'm running code, I expect to get a list of potential issues with it, regardless who wrote it. E_DEPRECATED can be a cause of security issues (consider mysql_'s deprecation in the future), and E_STRICT can be cause for unknown vectors, because the PHP people tend to test the odd cases less.

Logging is an important aspect, and I expect people to do it properly (at least those that really care about their system). In cases where logging turns out to be excessively verbose, I've implemented two systems to tone it down (one of which is pretty smart).

With that, I think we can close this for now, until further notice.

[SergeyBiryukov]

Related: #21183

[nacin]

Fixed by #21183, [21644].