Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:116764 Return-Path: Delivered-To: mailing list internals@lists.php.net Received: (qmail 32744 invoked from network); 2 Jan 2022 16:47:15 -0000 Received: from unknown (HELO php-smtp4.php.net) (45.112.84.5) by pb1.pair.com with SMTP; 2 Jan 2022 16:47:15 -0000 Received: from php-smtp4.php.net (localhost [127.0.0.1]) by php-smtp4.php.net (Postfix) with ESMTP id C6DBE1804A7 for ; Sun, 2 Jan 2022 09:53:59 -0800 (PST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on php-smtp4.php.net X-Spam-Level: X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,HTML_MESSAGE,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.2 X-Spam-ASN: AS15169 209.85.128.0/17 X-Spam-Virus: No X-Envelope-From: Received: from mail-lf1-f52.google.com (mail-lf1-f52.google.com [209.85.167.52]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by php-smtp4.php.net (Postfix) with ESMTPS for ; Sun, 2 Jan 2022 09:53:59 -0800 (PST) Received: by mail-lf1-f52.google.com with SMTP id g26so70738017lfv.11 for ; Sun, 02 Jan 2022 09:53:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=craigfrancis.co.uk; s=default; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=kMizCWsbsDRjP1M17UiLG8EwcW3pwJUMpXmOKnve+JI=; b=fgSuZJNKWq6ASroXINZaY7y89vIKULYiAwZuj848/clbsAqO951SO1krBUSeMSQLc/ KspNA3zOTxA1NJEvpzGPWIpynPLLlzKuuhYIQsY4pwwuXBhf1TFc51BA4WSNUMaX1mFC jvHvUJJIMCGsnJVw6saePHrefLRoeXBYfgDeQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=kMizCWsbsDRjP1M17UiLG8EwcW3pwJUMpXmOKnve+JI=; b=T7AX0jT5gz4V7vM/v8qFKJ7g6Dc5s2t0oCWHAHOAr1SrfOmQ+X4Z/Lxvq1gc9QuTxr LoEtZ4YafGIlN0a8BUcW52LpFlkwyn2Rb3us0fZdcsaAaeRvradwefSqdryyg1BXCCcl 7mb2MFjOiLqjbq6j4cpHkA4C072muVnkl25cSpc7MWc9zJnnfMXUmgSpiQE8qHzC9KoS LXMFH1UzGCEma2W7o1XA7VVwKUvisDnj+NIEUVohJEuXHT2sw6o6OE9H6Dinb80QWaR6 vi/qppHT9bGgjoIHwt4RMST0jwHZzrBk7Otl54/Gm+hiPEoqZ2k9SD/THgSCVqb0pG4L ZSuA== X-Gm-Message-State: AOAM5304ZpVTSDNIWvC+7dO5rsJi7kY2MpsvSDbI7eDzBDhAwCOcB4do UfIeNgPBREUw63GOrXwagS1c0TKXow92A4kFv4kQRA== X-Google-Smtp-Source: ABdhPJynGbc5QlBUwzWYtoeRy9zGnZbXg6//IiJ9e2vzRW48JzxXgmImj569C1m70HIMCb5s28H2jlFCzx6PAK8RqSw= X-Received: by 2002:a05:6512:323b:: with SMTP id f27mr36673495lfe.302.1641146037598; Sun, 02 Jan 2022 09:53:57 -0800 (PST) MIME-Version: 1.0 References: <1174b73a-9e79-15f1-7fd3-497ad4f5ef31@allenjb.me.uk> In-Reply-To: <1174b73a-9e79-15f1-7fd3-497ad4f5ef31@allenjb.me.uk> Date: Sun, 2 Jan 2022 17:53:46 +0000 Message-ID: To: AllenJB Cc: internals@lists.php.net Content-Type: multipart/alternative; boundary="000000000000988dac05d49d1675" Subject: Re: [PHP-DEV] Allowing NULL for some internal functions From: craig@craigfrancis.co.uk (Craig Francis) --000000000000988dac05d49d1675 Content-Type: text/plain; charset="UTF-8" Thanks Allen, Before I start, the RFC is still a Draft, and I'm trying to find a solution... I'll also use a bit of sarcasm, just to be entertaining/funny, so it's not a page of boring technical stuff, I'm not arguing or saying you're wrong, and I genuinely appreciate your views on this. On Sun, 2 Jan 2022 at 11:27, AllenJB wrote: > I think this RFC is trying to solve the wrong problem / only fixing one > small portion of the "immediate" problem without paying attention to the > bigger picture. > Dynamic properties do cause problems, and I'm convinced they should deprecated, whereas NULL is a useful value. e.g. when a value has not been set; and then providing NULL to a function like htmlspecialchars(), I don't think should be a problem. I feel that effort would be better spent on improving and encouraging > use of tooling such as the PHPCompatibility CodeSniffer ruleset. This one requires variable tracing from source to sink, not an easy task (it's why I noted "Psalm at level 3, with no baseline"). Neither PHPCompatibility or Rector have rules for this at the moment, and I'm pretty sure they never will; other than the really obvious ones, e.g. `str_replace('a', NULL, $name)`, which is clearly a terrible thing to do :-) As an aside, PHPCompatibility did the bad thing of using trim() on getConfigData(), which can return NULL: https://github.com/PHPCompatibility/PHPCompatibility/commit/ab7b0b82d95c82e62f9cffb0f1960f3ccbf0805e https://github.com/squizlabs/PHP_CodeSniffer/blob/d6a58bdd8707bab1032344f33d01c0627e0f3f97/src/Config.php#L1479 As noted in the RFC, while these don't take long to fix, finding them is tricky, it nearly always results in more code (yay?), and it is being seen as wasting time on a fairly questionable improvement. The migration guides could include documentation on updating existing > code for each change While migration guides are useful (I appreciate them), I'm focusing on this change to avoid unnecessary work. I don't think nullable variables should need to be explicitly cast to a string before calling these functions - doing so doesen't seem to make the code better (if anything, it's making it more complex). ... this RFC creates work for developers both in debugging code, and for > developers with > defensive programming styles to (re-)add checks to detect the exact > class of problem these deprecations / warnings highlight. > For example? e.g. `$name`, if it's set to NULL, why should `ucwords()` have a problem with that? Won't it cause extra work having to explicitly convert it to a string? Consider those who aren't using Static Analysis (or they are using the entry levels, with a baseline), it requires that everyone remembers that while their HTML form would normally provide this value, the user can edit the
in their browser DOM, or a browser extension could do something weird (e.g. a password manager), or a network issue results in a partial page load... and kaboom, future Fatal TypeError in the middle of processing? yay? party time? ...all the frameworks used in the example have an additional > default value parameter. This makes all of these (and the null > coalesce alternative) relatively trivial to update using a regular > expression find and replace, that most text editors can do. They use a default value of NULL so you can determine when the value has not been provided... should we tell them to stop doing this? default to returning an empty string instead? I'm not sure they will accept that one. Some of the suggested functions (parameters) to change are strange > selections in my opinion It's a draft, I just started with as many candidates as I could find, on the basis that it's easier to cut back if NULL can cause a problem. But as to your examples (gz compressing, bcmath, password_hash, and mail); they can all be provided with user input, and other sources of NULL. e.g. `password_hash()`, while I wouldn't advise anyone to use a blank password, or anything less than 8 characters, this function does accept it. And because passwords often come from a POST request, if the value is not present (framework function returns NULL)... well, assuming this deprecation indicates a future where this is not acceptable, what happens? future Fatal TypeError? This is why I have the "Future Scope" section - noting that some parameters could be updated (in a different RFC) to complain when an Empty String or NULL is provided. e.g. `setcookie()` already complains when `$name` is empty, and `strpos()` probably should complain when `$needle` is empty. > If this change were to be made, I think it would be better to severely > restrict the list to only the most commonly used functions. I'm fine with that, but what should be removed? Feel free to do this on or off-list, pull request, etc. If you can, I'd really appreciate examples where the NULL would cause a problem but an empty string is fine - i.e. assume developers will be lazy, and just use strval() to fix the hideous crime they have been committing. How would code that declares strict_types be affected by this RFC? Good question... personally I think a trim() on NULL should return an empty string, not a fatal error, so maybe we are looking at this problem in a slightly different way? Craig --000000000000988dac05d49d1675--