Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:105428
To: internals@lists.php.net
References: <CAFPFaMJEn=gjk5jAAxR2bF=4Mt3r0FJCm6mbOCReOmyybtAkkg@mail.gmail.com>
 <59cafbfb-2bb0-468c-458f-74bcac780e0f@korulczyk.pl>
 <004c01d4f09f$880ac320$98204960$@roze.lv>
 <CAFPFaM+QwXpm-mfwRtbT2sPB_C-9wvPELVTV5dHvizgmfUjMnw@mail.gmail.com>
 <004401d4faa3$60f83700$22e8a500$@gmail.com>
 <2f922f17-bc7c-313a-8f77-122e861995be@lsces.co.uk>
 <5741936F-B1F4-43C7-B815-F9D8030AC7BB@koalephant.com>
 <e5c10f59-bde9-dadc-e837-266f870b6358@lsces.co.uk>
 <B392E7C8-BBAC-48D2-BBBA-3D7C7EFD9595@koalephant.com>
 <e702a5cc-9567-0591-8f5e-b14aef0766b7@lsces.co.uk>
 <CAAfnsFU1B61qaPRAh5uNTWMALcV5xmae0Yb1yY9+QaAGLVGx7A@mail.gmail.com>
 <49A4B76C-4C62-4CBE-BA20-FBE56CA29AB0@cschneid.com>
 <CADyq6sL7m=A=pA1rNBg_CFOoMtfCZn50AfALh9jtLHvn9NRzmw@mail.gmail.com>
 <609E93CF-099B-446C-AD28-04F1D802C9F0@cschneid.com>
 <CADyq6s+6Gqaa6Werc0Vn=gKWrrqC71+ry1rpsPEpq4BSE4wq_Q@mail.gmail.com>
 <CAPKYkKz4u-D3ebVJCU8TgCuEGVVC28-YSj76sCU1TS4BaDjeBA@mail.gmail.com>
 <CAAfnsFUAY_O9JbDWMsntFKz1otTjE0BKYrsMif3DzG+zQpahxw@mail.gmail.com>
Message-ID: <85e05f77-e828-9634-adec-f7d798d9f0b8@phpgangsta.de>
Date: Thu, 25 Apr 2019 02:19:38 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101
 Thunderbird/60.6.1
MIME-Version: 1.0
In-Reply-To: <CAAfnsFUAY_O9JbDWMsntFKz1otTjE0BKYrsMif3DzG+zQpahxw@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Subject: Re: [PHP-DEV] [RFC] [VOTE] Deprecate PHP's short open tags
From: info@phpgangsta.de (Michael Kliewe)

Hi there,

Am 24.04.2019 um 21:07 schrieb Peter Kokot:
> Hello,
>
> On Wed, 24 Apr 2019 at 19:44, Chase Peeler <chasepeeler@gmail.com> wrote:
>> On Wed, Apr 24, 2019 at 1:27 PM Marco Pivetta <ocramius@gmail.com> wrote:
>>
>>> Run a fixer: they are out there, and they are extremely stable too.
>>>
>>> Also a good chance to finally take a look at code that has been rotting in
>>> a hard drive for too much time.
>>>
>> All of that takes time though. I have 6,787 short opening tags found. Even
>> if I use a fixer to generate a diff, or, to fix them and then examine the
>> diff in a pull request... that's going to take a LOT of time. It's going to
>> start getting messy if I find false positives and need to exclude changes.
>> It still doesn't address the impact of changes that aren't found. Are you
>> 100% positive that the fixers out there will catch EVERY single instance?
>> php-cs-fixer doesn't update <? inside of strings. If that's generating
>> dynamic code, then that's not getting caught.
>>
>> The output from php-cs-fixer just generated a diff file that was 161,000
>> lines long. But yea, that's something I can scan through real quick and
>> make sure nothing is going to get broken. No chance I'll miss something
>> either.
>>
>> I would LOVE to have the time to go through our legacy code and clean out
>> old stuff. We have a lot of technical debt that, while we aren't paying it
>> off at the moment, it's gaining any interest either. We also have plenty
>> that is. It's tough to justify putting off the stuff that is gaining
>> interest to focus on stuff that isn't so we can fix short tags.
>>
>> I don't work for a software company. I develop an internal application for
>> a non-software company. There are 2 other developers and enough work for 10
>> developers. It's going to be VERY hard for me to get management to approve
>> putting off projects that have a direct impact on our business in order to
>> upgrade to PHP 8 when that time comes. I think you are going to find that's
>> a pretty common occurrence, and the adoption rate of PHP 8 is going to be
>> VERY low. Especially when more and more user-friendly alternatives like
>> python and node are coming along. It was one thing when the options were
>> RoR, ASP.NET, and JSP. That's not the ecosystem anymore, and it's only
>> going to provide additional user friendly opportunities in the next couple
>> of years leading up to PHP8.
>>
>> Can someone PLEASE tell me the positive gains this RFC will accomplish that
>> justifies risking:
>> 1.) Losing market share
>> 2.) losing credibility
>> 3.) removing a large number of libraries that are fine from a technical
>> aspect, but will stop working due to the existence of <?
>> 4.) new security risks (code leaks that are more likely than the code leaks
>> the RFC seeks to prevent)
>>
>> And, please don't use the existence of code fixers to justify this unless
>> you are willing to demonstrate how it's quick and easy to go through a
>> 161,000 line diff file or are willing to 100% guarantee that the fixer will
>> not break anything, will not fix anything it shouldn't, and will not miss
>> anything it should fix.
>>
>>
>>
>>
>>
>> --
>> Chase Peeler
>> chasepeeler@gmail.com
> I've done few commits with about 8k files changed with very repeating
> changes like this and without breaking anything. It will take you
> about 30minutes... Let's say, one hour for also taking a break from
> all the scrolling the git diff and to return the merge the next day
> after another check or something like this.
>
> The change (speaking for 8k files using short opening tags converted
> to <?php) itself takes about 80 seconds with using PHP CS Fixer but
> the memory_limit needs to be increased a bit and only relevant change
> is done in all files. 8k (or 6k) opening tags is nothing very big
> actually.
>
> Then also search for similar occurrences by using a regex for those
> edge cases you're mentioning in strings and such (I doubt there are
> but ok) and you have this fixed. Then run this app on PHP 8 and you'll
> see the other BC breaks that happen due to the major release. Major
> releases are meant to break things because otherwise we can only load
> the language with new things and maintain old stuff in there for
> eternity.
>
> That's why serious apps have support cycles and follow their
> frameworks release and life cycle, and in the end also PHP cycle.
> Simple as that.
>
> --
> Peter Kokot


Some thoughts and numbers from my side.

Nobody should not use a simple grep or sed like this:

grep -rin "<?[^pxi=]" . | grep ".php"

It does not search in .phtml templates for example. And it has lots of 
false-positives.

This is the output in one of my projects:

# grep -rin "<?[^pxi=]" . | grep ".php"
./scripts/OpenInviter/autoupdate.php:97: 
$start=0;$end=strpos($res,"<?");$signatureBulk=trim(substr($res,$start,$end));$fileStriped=str_replace($signatureBulk,'',$res);
./scripts/XXXX_soap/XXXX_soap.php:1:<?
./scripts/XXXupload.php:67:        Text: <input type="text" name="text" 
value="<?echo $_REQUEST['text']?>"><br>
./scripts/XXXupload.php:68:        Text2: <input type="text" 
name="text2" value="<?echo $_REQUEST['empfaenger']?>"><br>
Binary file ./library/phpillow/tests/phpillow/data/image_gif.gif matches
Binary file 
./library/phpillow/tests/phpillow/data/.svn/text-base/image_gif.gif.svn-base 
matches
./library/ezcomponents/Mail/src/tools.php:225:        $pattern = 
'/<?\"?[a-zA-Z0-9!#\$\%\&\'\*\+\-\/=\?\^_`{\|}~\.]+\"?@[a-zA-Z0-9!#\$\%\&\'\*\+\-\/=\?\^_`{\|}~\.]+>?$/';
./library/XXXX/SoapApi.php:1:<?
Binary file 
./library/dompdf/lib/php-font-lib/sample-fonts/IntelClear-Light.ttf matches


Result:
2 false positives
3 results because "<?" seems to be in random binary files, and "php" is 
in the file path. Better check would be:
   grep -rin "<?[^pxi=]" . | grep "\.php"
2 places found where "<?echo " should be replaced by "<?=" or "<?php "
2 results with a short open tag at the beginning of a file

BUT: That simple grep did only find a small fraction of the problems, 
for example .phtml templates were not scanned. Continue reading...


php-cs-fixer with default settings produced a diff file with 3403 lines, 
51 line changes:
php php-cs-fixer-v2.phar fix --rules=full_opening_tag --diff --dry-run . 
 > result.txt
Reviewing these 51 changes took 3 minutes. No false positives here, all 
of them were at the beginning of files.

BUT: it didn't search in .phtml files. php-cs-fixer by default only 
processes .php and .phpt files (depending on the version you are using, 
older versions also process .twig files as far as I can see). If you use 
it, you may have to add other file extensions like .phtml, .tpl or 
similar, depending on your project.

Running php-cs-fixer on .phtml files in that project results in a diff 
file with 9527 lines, 1573 line changes just in the .phtml files of that 
project. Whao...


I'm not sure if I'm alone, but I have lots of template code like this:

<div class="tableBody">
<?  foreach ($this->Xdata as $index => $data) { ?>
         <div class="tableEntry <? if ($index%2 != 0) echo 
'odd';?>">Something</div>
<?  } ?>
</div>

Blindly replacing "<? " by "<?php " results in this:

<div class="body">
<?php  foreach ($this->Xdata as $index => $data) { ?>
         <div class="entry <?php if ($index%2 != 0) echo 
'odd';?>">Something</div>
<?php  } ?>
</div>

The nice visual indenting of the "foreach" with 4 spaces is broken now, 
and cannot be fixed, because "<?php " is 6 chars long...
Yes, minor problem of cause, but it looked nice before, and now looks 
broken and "wrongly indented".


Some random thoughts:
- What happens to .phar files? Do we have to scan the contents?
- What happens to "executable php files"? They don't have a file 
extension, but a shebang "#!/usr/bin/env php", and potentially 
short-open-tags in it.
- What happens if I miss to add a file extension to php-cs-fixer while 
scanning and fixing code I'm not 100% familiar with? Code leak :-(
- What happens if I upgrade a server to PHP8, but do not check all 
folders and projects on it?
- What happens if a Shared Hosting Provider increases the PHP version to 
8 automatically for all its customers in the future? Does this happen in 
the real world? Pages will not generate a 500 error, they will leak 
code, with passwords in it...
   Same with system administrators upgrading PHP to 8 without asking 
code-owners/developers. Or migrating all projects blindly to a new 
server with PHP8 installed. I guess that might happen. I guess, nobody 
expects code leaks then...

Looks like a lot of work and problems for everybody... I didn't expect 
the RFC to pass, thought that there are too many problems and lots of 
work for no benefit :-( And I don't follow internals mailing list on a 
daily basis, just check it "every few weeks for something important"...
In the past I was fine with BC breaks, for security, performance, or 
similar. Even bigger ones (unable to detect like "Uniform Variablen 
Syntax" for example). But this one looks heavy and dangerous for no 
benefit...

At least the code leaks must not happen. A Fatal error is a lot better 
than a code leak (and easier to find in the error log :-) )...

"Just use php-cs-fixer" sounds easy, but it's a lot more complex, see 
problems and thoughts above.

Michael