Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:115010 Return-Path: Delivered-To: mailing list internals@lists.php.net Received: (qmail 30874 invoked from network); 22 Jun 2021 08:57:41 -0000 Received: from unknown (HELO php-smtp4.php.net) (45.112.84.5) by pb1.pair.com with SMTP; 22 Jun 2021 08:57:41 -0000 Received: from php-smtp4.php.net (localhost [127.0.0.1]) by php-smtp4.php.net (Postfix) with ESMTP id 4331F1804C0 for ; Tue, 22 Jun 2021 02:15:55 -0700 (PDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on php-smtp4.php.net X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=no autolearn_force=no version=3.4.2 X-Spam-Virus: No X-Envelope-From: Received: from mail-vk1-f169.google.com (mail-vk1-f169.google.com [209.85.221.169]) (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 ; Tue, 22 Jun 2021 02:15:54 -0700 (PDT) Received: by mail-vk1-f169.google.com with SMTP id 184so4316617vkz.13 for ; Tue, 22 Jun 2021 02:15:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=basereality-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Pegfcwh33ZIkcAtjxFwpqgzsDA8dH/8z2eUpS8sHYQs=; b=V31aHEBdkFbuDbRh7wqmZNQ5jmi/LtvgjhrZOmUe6Bzu+hyOy6USUl4RD09aVSBY1t cCRtfgHX21LTHVdoxptbvZwNNQlij94XlDpRSFFGfgAHLBldS8SX6uglOSioBCLMc0mj gGU8QdpX2Y1trxOMRPNi2j2Fhy6Exfmi9TNG6FlGSsWwVUTzG5o/3HfdHyQGSNtA3Y7x j+a5b8DGTs5MoSNsKVnlGa6u+r6KdKrwKNk9ClyEpQ8MQ0sWcpGa4YXlnNSnYf1EnLcZ ooQMEXZxL3qbDPyOaTjLSZ6AmWudGqZBIqmWG+ID5JVbou31W1FZ3AgVVDviwgB7/Ieh HZ0A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Pegfcwh33ZIkcAtjxFwpqgzsDA8dH/8z2eUpS8sHYQs=; b=EU3pYtXbxzmGGIYDfGf+Ffhk3aGnaAcj4fcCOvyXs30fkSMdou7SXRcJ5pvumBDXKV Pan9i3la1j1HjITKZOHZ4TuxKPFMGXLb0cVEB++KAFD9pddu9wj+zBZJ8oOxZXiSEz3h ZD/Mv9KlWgzfV3BXs0XGozlZLgCSZIBKErU3XfQc8tdCMIWtVLe0wlmCT4+UaAoAv7AB bIFdbdIRt6wGp0vynwoHZNgc90m7A9/QkGRp++5BIIRhvjPQ7JeK1RXvgu+PKr/lhO0i HYW9NDcISwNntva9p0k6w/t6X7RzetFzASxznulvcWG7+WRHRL3z8uEFlVTvgNL9mqdV p/2Q== X-Gm-Message-State: AOAM5322cbcUgoawCZyIK2ZEUu4MY6F5bYSHcFO3k8J5PVh/1t5zh8ZS W8S5EEskWCQ+TFIp1O8KQ2vR/iPBpLl3/eJaTFIchg== X-Google-Smtp-Source: ABdhPJyVXCh/e2cOMhNxKNKEoKFw/vJ/NeXKOaYchggqZMr15+ZZOwgEa1CH1vX+U85+xOwB//FJBCWAdn0RtMFuUyA= X-Received: by 2002:a1f:2fc4:: with SMTP id v187mr18173953vkv.4.1624353353214; Tue, 22 Jun 2021 02:15:53 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: Date: Tue, 22 Jun 2021 10:15:40 +0100 Message-ID: To: Craig Francis Cc: Joe Watkins , Matthew Brown , PHP internals Content-Type: text/plain; charset="UTF-8" Subject: Re: [PHP-DEV] [RFC] is_literal From: Danack@basereality.com (Dan Ackroyd) Dan Ackroyd wrote: >> The developer who made that change would run the tests, >> see the tests pass and then push their code live. At which point, it >> would cause an error in production, as the string returned would not >> pass the is_literal check. Craig Francis wrote: > No, the only way fatal errors can occur is if you choose to make them fatal (in userland code). Please can you go into some detail about what you think people are meant to do when they detect a non-literal used where a literal is expected? This seems like a core flaw in the proposed RFC and I think you aren't addressing it. > that's going to make adoption very difficult. Why are you prioritising speed of adoption, over reducing the cost of using this feature for projects over say the next 5 or 10 years? It kind of sounds like the idea behind the RFC has been a compromise between the idea in Mr Kern's talk, and people who are not sure this feature is a good idea or not. This feature should be of most use to larger teams, where reasoning about the application is difficult to begin with. For them an extra few hours migrating a large code base to use the appropriate techniques is a fine trade-off over increased difficulty maintaining their code. > No, the only way fatal errors can occur is if you choose to make them fatal (in userland code). If people aren't going to make using a non-literal where a literal is expected, be an error, the only alternative I can see is logging it. Please correct me if you think people should be doing something other than those two things. From my experience, any process that involves looking in log files is a pretty bad process. For is_literal checks, what's going to happen is: * programmer makes a mistake, that isn't caught by a test. * programmer deploys site, and entries about non-literal strings start being written to log files. * someone then needs to remember to check the log files, and pull out the appropriate entries, and make a ticket for them to be investigated. * someone then needs to pick up that ticket, investigate the cause of the non-literal string being used, and trace it back to which module/library is causing it, then update the ticket for someone on that team to fix it. * someone on that team needs to have that ticket prioritised before they start work on it. Apologising for expressing myself poorly before, but this is a really, really, really, really, really, really result to have. It makes using this feature either have a high maintenance burden, and allows security flaws to exist until someone gets around to fix them, or it results in things failing in production. I know it's slightly annoying to require any combining of strings, where you want to preserve 'literal-ness', to have to jump through the hoop of using a specific function, but it's just a few seconds work, that would save hours of debugging. The whole reason why the solution worked for Google was that it made it cost less for programmers to do the right thing, rather than having to remember fix problems after they are found.* I don't think there would be an easy way to fix this if the RFC was passed in its current form. Changing from string concatenation carrying the literal flag around to not, would be a huge BC break, that would require multiple versions to fix. If it was the other way round so that we don't support carrying the literal flag around, and (much to my embarrassment) it does mean that people can't actually use the feature, because it's too difficult, it would be much easier to move to carrying the flag around. > However requiring developers to rewrite all of their code to use literal_concat() and literal_implode() No-one is forcing developers to rewrite their code. And I don't think many people would actually use those functions. The vast majority of people are going to continue to use the functions provided by Wordpress, Doctrine etc, and any checks on literal, or combining of literals would be internal to those functions. As I wrote to Thomas, I think the majority of people are more likely to shift to using type specific wrapping types, rather than copying bare strings around. cheers Dan Ack *For anyone wondering, unfortunately we can't use the same implementation as Google, due to PHP not being statically compiled, and also not being able to set 'type flags' on strings.