On 26 May 2022, at 20:06, Michael Babker <michael.bab...@gmail.com> wrote:
> On Thursday, May 26, 2022 at 11:41 AM, Craig Francis 
> <cr...@craigfrancis.co.uk (mailto:cr...@craigfrancis.co.uk)> wrote:
>> [...] If there is a good reason for throwing an exception when NULL is 
>> passed to `htmlspecialchars()`, then that reason would also apply to Laravel 
>> Blade for it's HTML encoding.
> 
> Laravel’s `e()` helper function is more than a wrapper around 
> `htmlspecialchars()` which supports values beyond a string, and has elected 
> to support a null value being passed into that function with explicit 
> handling for it. I’ve seen similar patches land in other packages as well. 
> Whether it is to prevent that package from triggering the deprecation 
> regarding null values itself or the package owners choosing to explicitly 
> handle the null case so users don’t need to deal with it doesn’t really 
> matter, the point is folks have made that decision and it is not problematic 
> in any way IMO. You’re basically saying that a framework choosing to apply 
> the same `htmlspecialchars($string ?? ‘’)` patch that applications are 
> suggested to use (for those who don’t care to differentiate between null and 
> empty string before something reaches an escaping function) is in the wrong 
> by arguing for Laravel to revert that patch.


Sorry, I've read this a few times, and I just don't understand what you mean... 
but let's leave it, I cannot think how else to re-write my question to get an 
answer.



On 26 May 2022, at 20:06, Michael Babker <michael.bab...@gmail.com> wrote:
> On Thursday, May 26, 2022 at 11:41 AM, Craig Francis 
> <cr...@craigfrancis.co.uk (mailto:cr...@craigfrancis.co.uk)> wrote:
>> 
>> `htmlspecialchars()` can and always has safely worked with NULL, the same 
>> with the other 335 parameters I've listed. But when I suggested changing 
>> these parameters to be nullable, it was not well received.
> 
> The arginfo for the `htmlspecialchars()` function, and if I’m reading 
> correctly (I’m no C developer so I could very well be misinterpreting things) 
> the internal `php_html_entities()` function which `htmlspecialchars()` calls, 
> does not declare null as a supported value for the `$string` parameter. 
> Meaning that depending on whether your code uses strict_types or not, either 
> calling the function with a null value triggers a type error or requires 
> implicit type coercision to convert the value to a type that the function 
> does support. This means to me that the `htmlspecialchars()` function does 
> NOT support null values and that passing null through only works because of 
> the reliance on implicit type coercision. That, to me, is a valid reason for 
> a type error to be raised; the code very explicitly requires a string and is 
> not written to process a null value.
> 
> A blanket suggestion to make every string parameter which implicitly supports 
> a null value nullable because they previously supported null through type 
> coercion IMO should be shot down. Instead, parameters which are emitting a 
> deprecation notice because they are receiving an unsupported null value 
> should be reviewed, and if that function can by design support null values, 
> those parameters should be updated to reflect nullability. IMO, 
> `htmlspecialchars()` is not a function which should expliciitly support a 
> null value and the present deprecation is correct.


Erm, I really don't wish to be rude, but have you read my RFC, and the problem 
I'm trying to solve... in short, the backwards compatibility issue for PHP 9.0.

Developers will need to spend a lot of time updating their code, which is 
important to avoid the fatal type errors, but I simply do not understand what 
the benefit for them will be.



On 26 May 2022, at 20:06, Michael Babker <michael.bab...@gmail.com> wrote:
> On Thursday, May 26, 2022 at 11:41 AM, Craig Francis 
> <cr...@craigfrancis.co.uk (mailto:cr...@craigfrancis.co.uk)> wrote:
>> On 26 May 2022, at 15:01, Michael Babker <michael.bab...@gmail.com> wrote:
>>> Yes, that means that there is a lot of work involved for folks between now 
>>> and the release of PHP 9.0 to either refactor code to avoid type errors or 
>>> to get the language to expand the supported types in appropriate functions, 
>>> but IMO that effort from all parties is worth it in the long run to ensure 
>>> language consistency (I really don’t think userland PHP and 
>>> internal/extension PHP should have vastly different behaviors, and type 
>>> coercion support based on where a function is designed is one of those 
>>> holes that needs filled) and to ensure all APIs explicitly declare what 
>>> types of data they support.
>> 
>> 
>> 
>> I don't think userland and internal functions should have different 
>> behaviours either (my RFC does say "keep user-defined and internal functions 
>> consistent").
>> 
>> To achieve that, for those not using `strict_types=1`, I'm simply suggesting 
>> that NULL coercion continues to work for internal functions, and we update 
>> user-defined functions to allow NULL coercion.
> 
> On the record, I personally disagree with the strict_types declaration’s 
> existence purely because it makes the runtime inconsistent based solely on 
> what file something was called from, plus there are way to bypass a 
> developer’s explicit opt-in to strict_types being enabled which makes it a 
> “mostly strict but there’s still a gotcha” declaration at best. I don’t think 
> this type of deep rooted behavioral difference improves PHP in any way, but 
> that ship has long sailed.


I believe George Peter Banyard is trying to work on this.



> With that said though, I disagree with exposing the internal type coercion 
> behavior to userland code. I think the userland behavior as it is today is 
> the correct behavior, even if I look at implicit type coercision as a bug 
> which will cause hard-to-identify production issues for somebody at some 
> point down the road. Let’s also be real here, a not-so-insignificant number 
> of PHP builds are powered by platforms which have minimal if any automated 
> testing and even less static analysis, meaning a static analyzer is not going 
> to be of much help to those builds to begin with because they’re using 
> non-analyzed code as a foundation. Changing the language in a way that makes 
> these already fragile platforms even more suseptable to hidden type-related 
> bugs is a bad idea. Personally speaking, I’ve been bitten by enough 
> type-coercion related bugs in my career (and continue to address new issues 
> arise thanks to a combination of poorly formed legacy data in the database 
> plus open source packages becoming stricter in the types they support) that I 
> would rather advocate for the language moving in a direction that makes 
> type-related issues harder to hit and it to be noisy when it does encounter 
> such issues; the changes in PHP 8.1 IMO are a step in the right direction.


Sorry, but I'm finding this hard to follow, it sounds like you don't want any 
type coercion... e.g. the string "5" to int 5 should just not happen without 
the developer explicitly converting the type (e.g. intval)... I disagree, but 
we can leave it at that.

Craig

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Reply via email to