Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:114640
MIME-Version: 1.0
References: <CAF+90c-7-ohb7-dqjN=jFguxTv_qegRNgZdEa4_vW-yK3sjvpA@mail.gmail.com>
 <CAESVnVr+qVP9=MufakqiRFWp1Ozt+yRYytB1m633HSnWLZeZzA@mail.gmail.com> <CAF+90c-LgBecV7Yj6md=6x7Hra_NWmL85t-jNF5BjBOmFys5qQ@mail.gmail.com>
In-Reply-To: <CAF+90c-LgBecV7Yj6md=6x7Hra_NWmL85t-jNF5BjBOmFys5qQ@mail.gmail.com>
Date: Thu, 27 May 2021 08:54:38 -0500
Message-ID: <CAESVnVprVw8Xo-98-x9THpDq_o-dai03Q_Fae_tMGgwro3=vDQ@mail.gmail.com>
To: Nikita Popov <nikita.ppv@gmail.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="0000000000004a58bd05c3501a05"
Subject: Re: [PHP-DEV] Escape \0 in var_dump() output
From: pollita@php.net (Sara Golemon)

--0000000000004a58bd05c3501a05
Content-Type: text/plain; charset="UTF-8"

On Thu, May 27, 2021 at 8:42 AM Nikita Popov <nikita.ppv@gmail.com> wrote:

> The most prudent and BC-safe thing would be to add another function
>> `var_dump_escaped()` or even to prefer/advise json_encode() when content
>> safety is relevant, but additional type information is not (most of the
>> uses of var_dump we have).  Unfortunately, this doesn't actually fix the
>> initial problem you stated, which is just getting useful data out of CI
>> failures.
>>
>
> Well, we had https://wiki.php.net/rfc/readable_var_representation a few
> weeks ago, which is basically a better var_dump() with escaping and all --
> but you can tell how it went ;)
>
>

I don't specifically remember that thread, but suddenly I feel like I can
guess everything I need to know about the outcome. :(


Rather than change the output at all, why not just have a post-process step
>> in our test runner that transforms the output in the test report?  Then we
>> could be as aggressive as we want, going as far as escaping all
>> non-printables plus backslash since at that point we're in it for the human
>> readability (and knowing specific byte sequences is essential) and we break
>> zero BC for anyone else?
>>
>
> Transforming the test output (such that the transformed output is checked
> in EXPECT) would indeed be a way to address the original motivation.
> There's two caveats though:
> 1. It does not address the issue for other uses of var_dump(). People
> being confused by var_dump((array) $object) output because of the null
> bytes in the property names is practically a PHP classic. Of course, fixing
> this would take away from the authentic PHP experience :)
>

True, but neither (IMO) would just escaping null byte, which again I regard
as a half-measure.  It has all the negative impact of breaking BC (your
point about extension phpts is a good one), without going far enough to
address the human-readability problem for many common cases (CR/NL chief
among them).

Insert Karate Kid "squash like a bug" reference here.  Escape it all, or
escape none of it.


> 2. Changes to run-tests.php behavior also affect extensions, and I think
> this kind of change would be even harder to address for PECL extensions
> than a simple var_dump() behavior change (where you can at least easily
> fork two versions of the file for the tests that need it).
>
>
Would it? The report at the end of the run is for human eyes, generally
speaking.   A human can interpret escaping (or not) as needed.
Also, we could make the post-processing optional.

run-test.php --escape-test-output

Then we use that mode in CI where we need git-friendly diffs, and it works
"normally" elsewhere.

-Sara

--0000000000004a58bd05c3501a05--