FROM: Re: [PHP-DEV] [RFC] is_literal()
And if it can be implemented as a PECL module, that would be more preferable to me than a core module of php.
If it was in core, having to support that feature may limit optimizations or implementation changes that could be done in the future.
Just wanted to address this comment which was made on another thread (I did not want to hijack that thread.)
A large number of PHP users have no control over the platform they run on, so the option to use PECL modules is a non-starter for them.
Here are several of those managed hosting platforms I speak of. Collectively they host a large number of WordPress sites, and Pantheon also host Drupal sites:
https://pagely.com/
https://wpvip.com/
https://wpengine.com/
https://kinsta.com/
https://pantheon.io/
Given that, if there is an option between a useful feature being added to core or left in PECL, I would vote 100% of the time for core, since working with WordPress on a corporate site I can rarely ever use PECL extensions.
#fwiw
-Mike
A large number of PHP users have no control over the platform they run on,
so the option to use PECL modules is a non-starter for them.
Thanks Mike,
Personally I agree, I would say PECL modules are not preferable for "useful
features"; simply because I try to keep my systems only using core PHP
features where possible (makes server admin easier).
As you mention working with WordPress, I've seen a couple of developers who
have taken examples like:
$posts = $wpdb->get_results("SELECT ... WHERE post_type='post'");
Then edited it to something dangerous like:
$posts = $wpdb->get_results("SELECT ... WHERE post_type='" .
$_GET['type'] . "'");
To guard against this, do you think that WordPress could update their
get_results() function to do something like:
public function get_results( $query = null, $output = OBJECT ) {
if (!is_literal($sql)) {
trigger_error('This is an unsafe $query, please use
$wpdb->prepare()', E_USER_NOTICE);
}
Perhaps with a better message; then, over the years, increase the warning
level?
I think that would be a very useful way of getting developers aware of
these dangers.
Craig
On Mar 21, 2020, at 5:59 PM, tyson andre tysonandre775@hotmail.com
wrote:
FROM: Re: [PHP-DEV] [RFC] is_literal()And if it can be implemented as a PECL module, that would be more
preferable to me than a core module of php.
If it was in core, having to support that feature may limit
optimizations or implementation changes that could be done in the future.Just wanted to address this comment which was made on another thread (I
did not want to hijack that thread.)A large number of PHP users have no control over the platform they run on,
so the option to use PECL modules is a non-starter for them.Here are several of those managed hosting platforms I speak of.
Collectively they host a large number of WordPress sites, and Pantheon also
host Drupal sites:https://pagely.com/
https://wpvip.com/
https://wpengine.com/
https://kinsta.com/
https://pantheon.io/Given that, if there is an option between a useful feature being added to
core or left in PECL, I would vote 100% of the time for core, since working
with WordPress on a corporate site I can rarely ever use PECL extensions.#fwiw
-Mike
A large number of PHP users have no control over the platform they run on, so the option to use PECL modules is a non-starter for them.
Thanks Mike,
Personally I agree, I would say PECL modules are not preferable for "useful features"; simply because I try to keep my systems only using core PHP features where possible (makes server admin easier).
+1
As you mention working with WordPress, I've seen a couple of developers who have taken examples like:
$posts = $wpdb->get_results("SELECT ... WHERE post_type='post'");
Then edited it to something dangerous like:
$posts = $wpdb->get_results("SELECT ... WHERE post_type='" . $_GET['type'] . "'");
Yes. That is all too common.
To guard against this, do you think that WordPress could update their get_results() function to do something like:
public function get_results( $query = null, $output = OBJECT ) { if (!is_literal($sql)) { trigger_error('This is an unsafe $query, please use $wpdb->prepare()', E_USER_NOTICE); }
Perhaps with a better message; then, over the years, increase the warning level?
I think that would be a very useful way of getting developers aware of these dangers.
Well...
IMO getting that in WordPress core is highly unlikely for two (2) reasons:
- Getting the WordPress team to adopt new features is about as difficult as getting the PHP community to agree on and add a new feature. IOW, it is a huge uphill battle.
I have many, many unfulfilled feature requests in WordPress' trac that are up to 8 years old or more even though on many of the tickets there appears to be universal interest. What I have found is that the WordPress team does not address anything unless they need it to implement the features that Matt has dictated should be in the next version of WordPress. Because of that, many very useful ticketsl sit there for years and years and do not get addressed.
Of course if a core team member with commit access has a pet feature they want to add, it typically gets slipped it with no fanfare and then we are stuck with it (cough Customizer cough)
I did recently get one ticket addressed about 11 months ago that was a subset of a ticket that is 9 years old (and sadly the OP on the older ticket has since died of cancer.)
But I think I invested probably 80 hours over that duration constantly bringing it up on Slack publicly and via DMs and on the ticket. My guess is they were just tired of hearing from me. And ALL my ticket was asking for was a damn hook that many plugins had already added because WordPress core had not:
https://core.trac.wordpress.org/ticket/47056
- More importantly — and a much higher bar — WordPress core supports PHP 5.6 and so core cannot contain any features that are in a later version of PHP.
And don't expect that to change any time soon. WordPress' minimum PHP requirements was notoriously 5.2.4 until March 2019 so it is unlikely they will bump the PHP requirement before March 2029.
(just kidding, but only a bit.)
Of course WordPress currently recommends PHP 7.3 and most (all?) managed hosts support that so userland developers are free to use newer version of WordPress on our own projects and in our plugins and themes.
https://wordpress.org/about/requirements/
But again, WordPress core cannot use any features anytime soon that are not in PHP 5.6.
BTW, I did not comment on your is_literal() proposal because I try not to comment on things where others are addressing concerns and where I do not feel very strongly about adding or avoiding the feature. I created this thread as a new thread to expliclty separate discussions because they are orthogonal and I did not want to imply that I supported is_literal() as currently proposed.
But since you brought it up let me comment on is_literal(). I recognize the problem I think you are trying to solve — to be able to guard against SQL injection attacks — but I don't think is_literal() is a viable solution for that problem.
-
Looking at my code most of my dynamic values use to compose SQL that I pass to $wpdb->get_results() is in variables, not literals. In fact I rarely use literals. So I don't see that it would help me (or many others?) much at all.
-
Focusing on it being literal or not seems to me to be focusing on wrong thing. Something could be non-literal but still be safe. And a code hack could introduce a tainted literal (but with a code hack all bets are off.) is_taint() makes more sense to me, but even then I am not sure it directly addresses the use-case.
-
I feel like we might be better to introduce functionality that addresses the exact problem of SQL injection and not one that dances around its edges. Maybe we need a MySQL class as an alternative to the mysqli functions that has a "safe" property and when that "safe" property is true you can't run DDL, TRUNCATE, or DELETE? Not sure how to protect against injection for UPDATE but maybe someone else has an idea?
-
Lastly, I think it would be better to specify the problem(s) you are trying to solve and then hash out potential solutions on the list rather than propose a specific one in advance, maybe even creating an ad-hoc working group to come up with a solution that is likely to be accepted?
Anyway, #jmtcw on is_literal().
-Mike
Craig
FROM: Re: [PHP-DEV] [RFC] is_literal()
And if it can be implemented as a PECL module, that would be more preferable to me than a core module of php.
If it was in core, having to support that feature may limit optimizations or implementation changes that could be done in the future.Just wanted to address this comment which was made on another thread (I did not want to hijack that thread.)
A large number of PHP users have no control over the platform they run on, so the option to use PECL modules is a non-starter for them.
Here are several of those managed hosting platforms I speak of. Collectively they host a large number of WordPress sites, and Pantheon also host Drupal sites:
https://pagely.com/
https://wpvip.com/
https://wpengine.com/
https://kinsta.com/
https://pantheon.io/Given that, if there is an option between a useful feature being added to core or left in PECL, I would vote 100% of the time for core, since working with WordPress on a corporate site I can rarely ever use PECL extensions.
#fwiw
-Mike
FROM: Re: [PHP-DEV] [RFC] is_literal()
And if it can be implemented as a PECL module, that would be more preferable to me than a core module of php.
If it was in core, having to support that feature may limit optimizations or implementation changes that could be done in the future.Just wanted to address this comment which was made on another thread (I did not want to hijack that thread.)
A large number of PHP users have no control over the platform they run on, so the option to use PECL modules is a non-starter for them.
Here are several of those managed hosting platforms I speak of. Collectively they host a large number of WordPress sites, and Pantheon also host Drupal sites:
https://pagely.com/
https://wpvip.com/
https://wpengine.com/
https://kinsta.com/
https://pantheon.io/Given that, if there is an option between a useful feature being added to core or left in PECL, I would vote 100% of the time for core, since working with WordPress on a corporate site I can rarely ever use PECL extensions.
#fwiw
-Mike
If at all possible, I advocate for implementing in userland. Of course, the specific is_literal/taint feature is special in this regard -- it can’t be implemented in userland.
IMO, PECL is an antiquated system that needs a successor, in much the same way Composer is the successor to PEAR. I think there are folks working on a solution for this, but I’m not sure where they are in their efforts. If we could make extensions as easy to package, distribute, and install (and load without root privileges) as Composer packages are, then I think we could say that PECL extensions are preferable.
Maybe FFI can help in this regard?
In the meantime, I agree with you that general-use language features that cannot be implemented in userland can serve the community best in the core, rather than in PECL, but their general utility will need to be weighed against their impact to the engine (i.e., if a feature slows down the engine, we can’t put it into core).
Cheers,
Ben
FROM: Re: [PHP-DEV] [RFC] is_literal()
And if it can be implemented as a PECL module, that would be more preferable to me than a core module of php.
If it was in core, having to support that feature may limit optimizations or implementation changes that could be done in the future.Just wanted to address this comment which was made on another thread (I did not want to hijack that thread.)
A large number of PHP users have no control over the platform they run on, so the option to use PECL modules is a non-starter for them.
Here are several of those managed hosting platforms I speak of. Collectively they host a large number of WordPress sites, and Pantheon also host Drupal sites:
https://pagely.com/
https://wpvip.com/
https://wpengine.com/
https://kinsta.com/
https://pantheon.io/Given that, if there is an option between a useful feature being added to core or left in PECL, I would vote 100% of the time for core, since working with WordPress on a corporate site I can rarely ever use PECL extensions.
#fwiw
-Mike
If at all possible, I advocate for implementing in userland.
I disagree with this in many cases; more on that below.
Of course, the specific is_literal/taint feature is special in this regard -- it can’t be implemented in userland.
Well, I broke off this thread on its own to decouple from the is_literal/taint feature.
Although I agree it is not a userland option.
IMO, PECL is an antiquated system that needs a successor, in much the same way Composer is the successor to PEAR. I think there are folks working on a solution for this, but I’m not sure where they are in their efforts. If we could make extensions as easy to package, distribute, and install (and load without root privileges) as Composer packages are, then I think we could say that PECL extensions are preferable.
Totally agree with all of that.
Maybe FFI can help in this regard?
I think that is not a viable solution because FFI can run unsafe code and can be disabled in PHP.ini. Given the former most (all?) managed hosts will disable FFI because — as PHP.net says on https://www.php.net/manual/en/intro.ffi.php:
"Caution: FFI is dangerous, since it allows to interface with the system on a very low level. The FFI extension should only be used by developers having a working knowledge of C and the used C APIs. To minimize the risk, the FFI API usage may be restricted with the ffi.enable php.ini directive."
I would love to see forward movement on this. I think what is needed for a binary extension that could be loaded in userland is some language or tool that can generate guaranteed-safe extensions, and one that will provide a significant performance gain.
So what are the potential options? These are the ones that come to mind:
-
Create a safe language similar to PHP but that uses LLVM to compile down to a binary form that could be loaded in userland. There are numerous existing languages currently in development that might be roped in to becoming this language. Jai that Robert Hickman mentioned comes to mind: https://inductive.no/jai/
-
Create a safe language similar to PHP but that compiles down to C source code that can then be compiled to create a binary form that could be loaded in userland. Zephir comes to mind as a language that might be co-opted for this (zephir-lang.com) , and you Ben were the one to mention this to me.
-
Explore Rust to see if there are ways to leverage its safety and limit it to only safe features compiled to create a binary form that can be loaded in userland (I have no clue if this is even possible.)
-
Explore GoLang to see how to leverage it to create a binary form that can be loaded in userland.
-
Look for another language that can be co-opted to use for creating safe binaries. Maybe a less well-known language where the language's team would be motivated to implement the specific features PHP would need to make this a reality (Julia?)
-
Embrace Web Assembly (WASM) as the extensibility mechanism for PHP. If we do then people can use many compiled languages to create WASM binary files such as C, C++, D, Rust, GoLang, Julia, etc. And there is already an extension to handle WASM in PHP:
https://github.com/wasmerio/php-ext-wasm
- And finally, I am sure there are other solutions that did not come to mind that someone else has considered?
BTW, I don't think we can offer a "safe" extensibility method that has the ability to manipulate PHP on the low level, unless of course PHP offered safe lower-level APIs. So is_literal()/is_taint() might still be off the table here for one of these extension mechanisms. But I could be wrong here and if so I hope someone will explain.
That said, of all the options that came to mind using WASM as a replacement for PECL seems like it would be the best solution, because:
- WASM is WC3 recommendation
- WASM is designed to be safe: https://webassembly.org/docs/security/
- WASM has a package manager (wapm.io) so there are/will be a lot of existing WASM packages for use in PHP
- WASM compiles to a binary, so could be uploaded to a server and loaded as a binary by PHP
- As noted above, many languages can create WASM: github.com/appcypher/awesome-wasm-langs
As for performance, WASM is probably faster than PHP, but we'll need benchmarks to know how much faster.
So what do others think? Should we consider adding WASM support to PHP?
Maybe this would be a good example of implementing a proof-of-concept container to let people try it out and see what they think?
In the meantime, I agree with you that general-use language features that cannot be implemented in userland can serve the community best in the core, rather than in PECL, but their general utility will need to be weighed against their impact to the engine (i.e., if a feature slows down the engine, we can’t put it into core).
Agreed. If the feature slows down the engine then yes, it is problematic to put into core.
Of course we should be careful to guard against a small subset of people against a useful feature using a naive implementation that slow down core as an argument against the feature when it is possible that an intelligent implementation can exist w/o affecting performance.
And finally, as promised above, I disagree that everything should be pushed to userland whenever-possible.
I believe there is a great benefit to standardization of functionality. The benefits of said standardization include:
-
Avoids reinventing the wheel. While developers generally adopt core language features they re-invent the wheel in userland. Lack of awareness of existing, non-invented-here syndrome, dislike of the API, company dictates only to use approved external code, etc. This reinventing causes fragmentation and can result in a massive duplication of error.
-
Minimizes training/learning required. A developer learns the core feature and they are done. In userland they will have to be trained on or learn the new feature every time they use a different codebase or package that implements differently.
-
Allows commonality in articles, tutorials and documentation. If a standard feature exists then it can be used in any article, tutorial or documentation without necessarily having to show and explain a userland implementation. This helps to level-up the skill of the average PHP developer simply because more and better learning material will naturally become available.
-
Minimizes dependency hell and/or duplicated code to maintain. If a feature is in core then any PHP package can use it w/o having to bring in yet another dependency and/or maintain yet another userland implementation.
-
Empowers future developers to solve greater problems. Standardized functionality empowers future developers to solve new problems rather than continue to recreate solutions and/or manage the dependencies where others solved them. Think of the biblical Tower of Babel and how it stalled progress once everyone was speaking a different language.
-
The nature of programming languages. Programming languages are tools that embrace and simplify well known patterns so we can solve harder and harder problems. If everyone in the past had the opinion that code should be implemented in userland if-at-all-possible then we would all be coding everything in assembly language, still. So when we identify a common pattern I believe is should be moved out of userland and into the core.
In closing, I agree that 80+% of functionality that is used < 20% of the time can and should stay in userland. But for everything else — the ~20% of functionality that is used 80+% of the time, such as str_contains() — that should be moved out of userland and into PHP core. IMO.
#jmtcw #fwiw
Cheers,
Ben
-Mike
IMO, PECL is an antiquated system that needs a successor, in much the same way Composer is the successor to PEAR. I think there are folks working on a solution for this, but I’m not sure where they are in their efforts. If we could make extensions as easy to package, distribute, and install (and load without root privileges) as Composer packages are, then I think we could say that PECL extensions are preferable.
In case anyone reading this cares to do anything about this situation,
there are a few key issues with PECL today:
- Its codebase tied to PEAR, which has been deprecated. For PECL to
survive it needs to separate itself from PEAR to some degree, and it
also needs maintained. - It doesn't work on Windows.
- It isn't designed to work with ini configuration directories, and
instead is focused on changing a singularphp.ini
file.
However, PECL is actually a much better tool than I realized. It
supports many things that are useful for package authors, but they
don't seem to be used much -- my only guess is that they aren't well
known or documented:
- It can replace things in files, for example bumping version numbers
in source files. - It supports post-install scripts, although they run separately from
pecl install
. - It can install PHP files into the PEAR directories, which allows you
to ship PHP files with an extension. I'm hoping that over time with
preloading and other features this becomes a viable model, but it's
still useful today.
There is a tool called pickle that is intended to work on Windows and
Linux. It doesn't seem recently maintained (but probably still in
better shape than pecl). It has too much abstraction for my taste, so
I didn't look at its source a lot; maybe someone else can comment
more.
Le 22/03/2020 à 00:15, Ben Ramsey a écrit :
IMO, PECL is an antiquated system that needs a successor,
The tool, probably
BTW, the "forge" or "package registry" still make lot of sense
and IMHO have lot of value.
Remi
FROM: Re: [PHP-DEV] [RFC] is_literal()
And if it can be implemented as a PECL module, that would be more preferable to me than a core module of php.
If it was in core, having to support that feature may limit optimizations or implementation changes that could be done in the future.
Just wanted to address this comment which was made on another thread (I did not want to hijack that thread.)A large number of PHP users have no control over the platform they run on, so the option to use PECL modules is a non-starter for them.
Even if a module is "bundled", that doesn't mean it automatically
becomes available to all vanilla builds of PHP.
I know that a lot of Linux distributions break up their PHP packages so
that pretty much every module can be installed separately; conversely,
they provide pre-compiled packages for a lot of PECL and third-party
extensions. So there's very little difference between installing, for
example, "php-zip" (bundled) and "php-redis" (PECL).
I glanced at a couple of the WordPress hosts you mentioned, and couldn't
find a clear description of their PHP configurations, but I suspect it's
similar - some "bundled" extensions won't be enabled (e.g. various
database drivers), and some PECL extensions will be installed as
standard (e.g. Redis).
In practice, I think there are four types of extension, not two:
A) Core, and either can't be disabled, or disabling is very rare
B) Bundled, but optional; may be packaged separately; may be optional or
not supported by managed hosting
C) Not bundled, but widely available pre-built as well as through PECL;
may be included or available in managed hosting
D) Not bundled, and not widely pre-built; mostly available only via PECL
Type B and C enjoy similar availability to users, but type C allows a
package maintainer more freedom to maintain and evolve the extension;
type B trades that for a rubber-stamp of official approval.
Regards,
--
Rowan Tommins (né Collins)
[IMSoP]
Le 21/03/2020 à 23:52, Mike Schinkel a écrit :
FROM: Re: [PHP-DEV] [RFC] is_literal()
And if it can be implemented as a PECL module, that would be more preferable to me than a core module of php.
If it was in core, having to support that feature may limit optimizations or implementation changes that could be done in the future.Just wanted to address this comment which was made on another thread (I did not want to hijack that thread.)
A large number of PHP users have no control over the platform they run on, so the option to use PECL modules is a non-starter for them.
Sorry, but PECL is the usual way for new extension
- add to pecl
- people start using it
- if success add to php-src
And later
- move to pecl extensions removed from php-src
Having new extension (outside of language feature) directly added
to php-src is IMHO a terrible way.
Having "dead" extensions in php-src is a real problem
(the ones nobody take care of anymore, only adapted for new version)
But I'm probably the only one who think much more extensions
should be removed from php-src and maintained outside, in pecl.
Remi
Le 21/03/2020 à 23:52, Mike Schinkel a écrit :
FROM: Re: [PHP-DEV] [RFC] is_literal()
And if it can be implemented as a PECL module, that would be more preferable to me than a core module of php.
If it was in core, having to support that feature may limit optimizations or implementation changes that could be done in the future.Just wanted to address this comment which was made on another thread (I did not want to hijack that thread.)
A large number of PHP users have no control over the platform they run on, so the option to use PECL modules is a non-starter for them.
Sorry, but PECL is the usual way for new extension
- add to pecl
- people start using it
- if success add to php-src
And later
- move to pecl extensions removed from php-src
Yes, it is the usual way. Which from my perspective is flawed because it means that a large population of PHP developers can never use those extensions.
Having new extension (outside of language feature) directly added
to php-src is IMHO a terrible way.Having "dead" extensions in php-src is a real problem
(the ones nobody take care of anymore, only adapted for new version)
That is not the argument that was made. The argument is that a developer cannot:
- Depend on a PECL extension to exist, and
- Cannot even use (most) PECL extensions when they use managed hosting.
So the PHP extensions mechanism is in large part the problem here. If PECL extensions were 100% safe and could be loaded by userland using only PHP code then most of the pressure to see functionality is core could be relieved. That's why I floated the idea of supporting WASM in core.
But I'm probably the only one who think much more extensions
should be removed from php-src and maintained outside, in pecl.
Let me guess. You or your team manages your PHP servers, you don't use managed hosting?
-Mike
Le 21/03/2020 à 23:52, Mike Schinkel a écrit :
On Mar 21, 2020, at 5:59 PM, tyson andre <
tysonandre775@hotmail.com> wrote:
FROM: Re: [PHP-DEV] [RFC] is_literal()And if it can be implemented as a PECL module, that would be
more preferable to me than a core module of php.
If it was in core, having to support that feature may limit
optimizations or implementation changes that could be done in
the future.Just wanted to address this comment which was made on another
thread (I did not want to hijack that thread.)A large number of PHP users have no control over the platform
they run on, so the option to use PECL modules is a non-starter
for them.Sorry, but PECL is the usual way for new extension
- add to pecl
- people start using it
- if success add to php-src
And later
- move to pecl extensions removed from php-src
Yes, it is the usual way. Which from my perspective is flawed
because it means that a large population of PHP developers can never
use those extensions.
Bundling doesn't make much of a difference. Only force-enable (like we
do with ext/standard, date, SPL and few others do) does.
Most users use distribution-provided packages of PHP. For those Core or
PECL matters little. Distributors package extensions in seperate
packages, even if we consider them core. At the same time they ship
pecl extensions. Thus whether one wants gd or some pecl extension is
the same complexity.
For somebody building PHP themselves compiling a PECL extension isn't
too difficult either.
For Windows pecl produces builds where we can, while users have to
install by hand.
Awarness however is different ...
johannes
For Windows pecl produces builds where we can, while users have to
install by hand.
Yeah, I've noticed this and thought about building a tool to help
automate installation.
However, it would be much easier to use PECL extensions on Windows if
PHP on Windows actually had a simple way to add all the DLLs in a PECL
extension to just the 'ext' directory and not have to copy multiple
DLLs all over the place.
For example, imagick has a ton of imported DLLs that are referenced by
the main extension DLL, which fails to load for a lot of people. Then
they have to go searching Google to figure out how to fix it for each
separate SAPI.
I'm thinking the problem is something that can be remedied with
SetDllDirectory():
https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-setdlldirectorya
Or AddDllDirectory():
https://docs.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-adddlldirectory
Depending on if WinXP support is still a thing and/or having multiple
directories is even possible. Calling LoadLibraryEx() looks like it
would be a pain to implement given that the only references in the
entire code to "LoadLibrary" is pushed all the way back into m4/configure?
As PHP parses the 'extension_dir' INI option, it could call the relevant
function above to allow for loading additional DLLs from the same
directory when loading extensions. Doing that would fix so many Windows
loader related issues that people have with getting the precompiled PECL
extensions to work on Windows (especially with mixed SAPI environments).
--
Thomas Hruska
CubicleSoft President
I've got great, time saving software that you will find useful.
And once you find my software useful: