[php-src] Issue #21643: OpenSSL extension has many inconsistencies in how errors are handled/reported/propagated

[email protected] (ndossche)
Newsgroups php.bugs
Message-ID <[email protected]>
Issue: https://github.com/php/php-src/issues/21643
Author: ndossche

### Description

The OpenSSL extension has a number of error-handling inconsistencies that I'm unsure how to handle. We should come to a consensus on how to move forward.
Inconsistencies include:
* Failure reason is not properly communicated to the caller of the function, or can't be distinguished at the call site.
* Some functions emit warnings, some don't.
* Some error-handling paths call `php_openssl_store_errors()`, some don't.
* Some error-handling paths call `php_openssl_store_errors()` but still return a success.

This means that partial success isn't obviously reported to the user in some cases, so theoretically they'd always have to check the `openssl_error_string()` function even if the function returned success, which isn't great.

## Some examples

Partial failure example (error stored but loop not aborted, may be intentional but this isn't communicated):
https://github.com/php/php-src/blob/1a9a2c7052e3e22b21c43b31dda9355b6d975f38/ext/openssl/openssl_backend_common.c#L97-L99

---

The following two calls may fail because of an inexistent file or some I/O failure
https://github.com/php/php-src/blob/1a9a2c7052e3e22b21c43b31dda9355b6d975f38/ext/openssl/openssl_backend_common.c#L1293-L1297

but at least one of the callsites can't distinguish between different kinds of failure:
https://github.com/php/php-src/blob/1a9a2c7052e3e22b21c43b31dda9355b6d975f38/ext/openssl/openssl.c#L940-L944

---

Very similarly to the previous one, different kinds of I/O or validation failures are not distinguished here:

https://github.com/php/php-src/blob/1a9a2c7052e3e22b21c43b31dda9355b6d975f38/ext/openssl/openssl_backend_common.c#L511

---

Example of silently swallowing errors without returning a different return value or without emitting a warning; even though this same function emits warnings for other cases:

https://github.com/php/php-src/blob/1a9a2c7052e3e22b21c43b31dda9355b6d975f38/ext/openssl/openssl_backend_common.c#L827-L838

---

There's a bunch of calls to `php_openssl_conf_get_string` that silently ignore errors: because e.g. the key may be optional but note that a configuration error can also result in a silent failure.

For example here:
https://github.com/php/php-src/blob/6a4d0d9456bf872286722ad5f043ad4928e92c9a/ext/openssl/openssl_backend_common.c#L254-L257

but also in various spots in https://github.com/php/php-src/blob/6a4d0d9456bf872286722ad5f043ad4928e92c9a/ext/openssl/openssl_backend_common.c#L293

etc

---

These are just some examples, but shows a much broader problem with the error-handling design of the extension.

### PHP Version

```plain
Report was made on master, but applies to all versions really.
```

### Operating System

_No response_
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.