Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:114647
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>
 <CAESVnVprVw8Xo-98-x9THpDq_o-dai03Q_Fae_tMGgwro3=vDQ@mail.gmail.com> <CAF+90c-C1nNYn=CB8ZyC2o_y+fU8z8UNKU=67QkMqhmb3c8i-Q@mail.gmail.com>
In-Reply-To: <CAF+90c-C1nNYn=CB8ZyC2o_y+fU8z8UNKU=67QkMqhmb3c8i-Q@mail.gmail.com>
Date: Thu, 27 May 2021 11:11:01 -0500
Message-ID: <CAESVnVqL+k_+htAmjMMDPDT0iR1e4GW8UhmYfOSsVFrKLQoC6g@mail.gmail.com>
To: Nikita Popov <nikita.ppv@gmail.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000024a1405c35202eb"
Subject: Re: [PHP-DEV] Escape \0 in var_dump() output
From: pollita@php.net (Sara Golemon)

--000000000000024a1405c35202eb
Content-Type: text/plain; charset="UTF-8"

On Thu, May 27, 2021 at 9:07 AM Nikita Popov <nikita.ppv@gmail.com> wrote:

> Ah, I think I explained the original issue badly: The test runner output
> isn't really a problem, or at least I never perceived it to be.
>
> The problem is that if you change a test that contains a null byte
> anywhere (read: in var_dump output), then github will not show a diff for
> that file. You can see this nicely in the proposed PR itself:
> https://github.com/php/php-src/pull/7059/files Most of the updated test
> files show up as "Binary file not shown." As you can imagine, this makes
> review hard. Using the .patch file doesn't help either, because it only
> contains entries like:
>
>
AH! Yeah, I completely misunderstood the problem space.  I didn't imagine
for a second we had non-ASCII in any test because that's bad and it should
feel bad.

Okay, then I still disagree with changing var_dump()'s output, but for
different reasons.

So take Zend/tests/bug60569.phpt for example, which has this EXPECT section
(where ^@ is actually the null byte):

--EXPECT--
string(20) "Some error ^@ message"

Instead of changing how var_dump() works in order to produce different
output, I'd change the EXPECT to and EXPECTF and add a new %0 pattern

--EXPECTF--
string(20) "Some error %0 message"

This creates zero BC breaks, allows extensions to continue working just
fine with raw nulls or update to use %0 if they'd like, and creates no
ambiguities.

-Sara

--000000000000024a1405c35202eb--