feat: allow empty extension name, fixes #19812

[shyim]

Allows to load a PHP extension by a php.ini like:

extension=${PROFILER}

If PROFILER env is not there, it loads nothing. if there. it loads that extension

[henderkes]

Huh, that's a really cool idea.

[iluuu1994]

Looks reasonable. Do you mind shooting a quick e-mail to the internals mailing list to ask for feedback? Technically this may still be considered for 8.5 RC1.

[shyim]

I did :) https://news-web.php.net/php.internals/128687

[hakre]

This is already the case. The empty string is ignored, that's why you see the diagnostic message.

What does it diagnose and give you the warning for the benefit: the empty string. It's just not necessary -- don't send an empty directive, it's void. The warning only tells you this already (and not the three+ other permuations that are possible for a non-empty stirng)

The only alternative I can think about is existing with code 254 to fail fast because setting a directive to empty should be diagnosesd as E_DEPRACATED like a since PHP 8.4 function parameter that is just = null without telling per it's type that NULL is included for it's value. Directives are string per their type per the INI directives (at least never NULL) and henceforece PHP 8.4 should have given an E_DEPRECATED error, therefore master should already have rejected with a non-zero status code (likely 254 as it's early) and shutdown.

My 2 cents.

[hakre]

And to add this report is wrong for the extensions named, the directive is zend_extension not extension (as given).

[iluuu1994]

@hakre

This is already the case. The empty string is ignored, that's why you see the diagnostic message.

What do you mean? This is the same message shown for any non-existent extension.

don't send an empty directive, it's void.

Did you read the original issue? The aim is to have something like:

{if hasEnv("FOO")}
extension=foo.so
{/if}

without having to invent new syntax. Tim suggested a simple approach of ignoring empty extension directives. As mentioned on the list, if the fear is that we might accidentally pass empty environment variables without warning, we could use a different sentinel value. Inventing a new if conditional would be significantly more involved, though also solve more problems.

[hakre]

What do you mean? This is the same message shown for any non-existent extension.

Yes, right, and rightly so. If you give PHP the directive to try a null string for the directive it resolves it as a relative pathname. PHP does not stop to load here. Don't provide it if you don't mean it.

Did you read the original issue? The aim is to have something like:

{if hasEnv("FOO")}
extension=foo.so
{/if}

Yes I did, this is the equivalent of

.PHONY : conditional
conditional :

# If people set these on the command-line, use them
PHP ?= php
PHPFLAGS ?=

ifdef FOO
  PHPFLAGS += --define extension=foo.so
endif

conditional :
	$(PHP) $(PHPFLAGS) -r 'printf("Hello, world!\n");'

Biased towards an in-work-tree invocation, and it shows more clearly what the extension directive stands for: load the extension and with it, load the extensions' directives. This makes the two directives, extension and zend_extension, special.

The equivalent at the entry-point of a system image packaged FROM the Docker library is much shorter and works since PHP 5.4:

if [ "$FOO" ]
then
  PHP_INI_SCAN_DIR="$PHP_INI_DIR/cfg-foo:$PHP_INI_SCAN_DIR"
fi

Note the colon ":" in the listing; the empty entry stands for the compile time value (ex. "$PHP_INI_DIR/conf.d" in a PHP container from the library). You may want to flip the order (append instead of prepend) or entirely set/replace.

Table: Environment Parameter at the Entry-Point affecting PHP Resource Configuration and Scanning for INI-Files

Parameter	Configure Build Argument	Comment
PHPRC	--with-config-file-path=PATH	php.ini 1-20
PHP_INI_SCAN_DIR	--with-config-file-scan-dir=PATH	>= 5.4, bc6a9ad

To complete the example:

$ cat "$PHP_INI_DIR/cfg-foo/foo.ini"
extension = foo.so

The OPs' configuration directive, now conditionally parsed based on the FOO environment parameter.

Problem solved. Otherwise, there is /tmp in a read-only container.

Nevertheless, to be clear: Our problem is not a read-only file-system here, from a systems engineering perspective we were longtime coming from the standpoint that files in sysconfigdir are not changed by the user nor the program. This is also the reason why the OP's-known solution actually is technically wrong when packaging the system image, and we should avoid any potential x/y problems here.

without having to invent new syntax. Tim suggested a simple approach of ignoring empty extension directives. As mentioned on the list, if the fear is that we might accidentally pass empty environment variables without warning, we could use a different sentinel value. Inventing a new if conditional would be significantly more involved, though also solve more problems.

Yes, right, and if we still want to patch the configuration loading I'd recommend to add two new directives:

1. extension_ifdef
2. zend_extension_ifdef

they only delegate to their sibling if the value on the right-hand side represents a non-empty string; that is:

1. not "" and
2. not false and
3. not null

This helps to clearly differentiate between their ground form and the intended use, that is to achieve the command-line directive semantics as outlined in the Makefile listing for PHPFLAGS above.

There is also no need to find a sentinel value; I think empty is the best value for what we want to express here, especially as we have to map environment parameter effective and efficiently.
And the two new proposed directives are speaking a clear language. As those are new directives, they should work on the command line, too.

Other directives should not be affected: we use extension and zend_extension to shortcut the effectiveness of the extensions configuration directives (if it could be loaded) so we move the condition to the place where it belongs in the logical order of the abstractions.

flowchart LR
    A((start))
    A --> C
    A --> d1
    A --> d2
    d1(extension_ifdef) ---> B
    B{defined?} -- no --> C
    C((stop))
    B -- yes --> d2
    d2(extension)
    d2 --> C

Loading

---

Furthermore, and additionally I propose we should add another long option and an ini-file counterpart.

The --define-env <name>=<envvar> long option that is similar to --define <name>=<value> and set the directive <name> where <envvar> is the name of an environment variable from which to retrieve the value. Unlike -d there is no shortcut for directly setting the value to an empty string, instead the environment variable itself must be set to the empty string. It is an error if the <envvar> does not exist in the environment. (copied from git(1))

[PHP]

example_directive:env = ENVVAR

The right hand side evaluates to the name of an environment variable. It is an error if it does not exist in the environment.

Consequences of an error in these instances: The runtime stops loading the configuration and exits with a non-zero exit status not larger than 254, including.

To share the benefit of the new error handling within the configuration, I'd further-furthermore and additionally suggest to have ':?'/'?' semantics with the environment parameter expansion in php-ini-files:

example_directive = ${ENVVAR?}

Given ENVVAR does not exist in the environment, it is an error with the same consequences as already outlined.

example_directive = ${ENVVAR:?}

Given ENVVAR does not exist in the environment or is null, it is an error with the same consequences as already outlined.

[bukka]

I read this and I'm not really sure what's the best solution. Empty and sentinel seems a bit hacky (sentinel is probably better) and conditionals seem quite complex.

What is clear that there is really no clear agreement so I think this needs RFC.

[bukka]

What I'm thinking is that it might be best to do RFC for empty value or sentinel and if it doesn't pass, then it would be a signal that we should go for something like conditionals.

I'm quite undecided and wouldn't feel comfortable to just merge this as it's kind of a subjective thing. Even though I thin it's useful.

[hakre]

@bukka: posting to the list can't hurt, and thinking about alternative suggestions: we could ask the OP if using the existing PHP_INI_SCAN_DIR / PHPRC configuration options leviated or even solved the problem to get a better picture how grave the need is. @shyim

Additionally my current understanding is that an empty value for the extension option is relatively safe (but only if it is empty). Rationale for that is an empty pathname is never a pathname of a file (only a directory), and for a file we are looking here.

If a user wants to safeguard and protect against an unintentionally empty or undefined environment variable, they can use the default value fallback available since PHP 7.2 or so:

[PHP]

extension = ${MY_VAR:-XXX}

As the extension XXX is not available, they will see the warning as they would have seen that for empty. This was the critical path for me that we shoot the messenger but we would not here.

Some red-teaming on this would be great as now I've let my shields down.

On the topic of conditional syntax:

I would rather not suggest to do them early or for this one-time-need (very limited number of affected directives vs. all parameters/variables, this grows out of proportion/balance very easily and needs even more planning if not even no-saying).

For an alternative future benefit (but out of scope in this PR/Issue) adding support for the earlier mentioned ${MY_VAR:?} to error on an empty or undefined environment variable (or cfg option) are likely a better candidate. (Cf. #20060)

The extension_ifdef / zend_extension_ifdef I still think is one approach, but as written, it is limited and my educated guess is that working with parameter substitution in the form of ${...:-} (and perhaps in the future ${...:?}) is a better and more portable concept. Also easier to learn and adopt to in terms of parametrization as prior art exists and it leans on the Shell Command Language's Parameter Expansion https://pubs.opengroup.org/onlinepubs/9799919799/utilities/V3_chap02.html#tag_19_06_02.

My 2 cents.