Data::Dumper: Malformed UTF-8 character since 5.33.8

[eserte]

The following script indicates a problem since 5.33.8:

use strict;
use warnings;
use Data::Dumper;
use Devel::Peek;

my $utf8_qr = "\x{20ac}"; $utf8_qr = qr{$utf8_qr};
my $s = [$utf8_qr, "\344"];
my $d = Data::Dumper->new([$s])->Dump;
Dump $d;
#binmode STDOUT, ':utf8'; print $d;

Output is (with 5.33.8, 5.33.9):

SV = PV(0x56407351a490) at 0x5640733eb400
  REFCNT = 1
  FLAGS = (POK,pPOK,UTF8)
  PV = 0x56407341c9a0 "$VAR1 = [\n          qr/\342\202\254/u,\n          '\344'\n        ];\n"\0Malformed UTF-8 character: \xe4\x27\x0a (unexpected non-continuation byte 0x27, immediately after start byte 0xe4; need 3 bytes, got 1) in Dump at /tmp/dd.pl line 11.
 [UTF8 "$VAR1 = [\n          qr/\x{20ac}/u,\n          '\x{0}        ];\n"]
  CUR = 55
  LEN = 64

With 5.33.7 it looks OK:

SV = PV(0x55dd1ca94ec0) at 0x55dd1c966400
  REFCNT = 1
  FLAGS = (POK,pPOK)
  PV = 0x55dd1c975960 "$VAR1 = [\n          qr/\342\202\254/u,\n          '\344'\n        ];\n"\0
  CUR = 55
  LEN = 64

Printing the dumped string gives me a � instead of a-umlaut.

I suspect this could be due to #18619 --- @arc, will you take a look?

[khwilliamson]

On 5/3/21 8:41 AM, Slaven Rezić wrote: The following script indicates a problem since 5.33.8: |use strict; use warnings; use Data::Dumper; use Devel::Peek; my $utf8_qr = "\x{20ac}"; $utf8_qr = qr{$utf8_qr}; my $s = [$utf8_qr, "\344"]; my $d = Data::Dumper->new([$s])->Dump; Dump $d; #binmode STDOUT, ':utf8'; print $d; | Output is (with 5.33.8, 5.33.9): |SV = PV(0x56407351a490) at 0x5640733eb400 REFCNT = 1 FLAGS = (POK,pPOK,UTF8) PV = 0x56407341c9a0 "$VAR1 = [\n qr/\342\202\254/u,\n '\344'\n ];\n"\0Malformed UTF-8 character: \xe4\x27\x0a (unexpected non-continuation byte 0x27, immediately after start byte 0xe4; need 3 bytes, got 1) in Dump at /tmp/dd.pl line 11. [UTF8 "$VAR1 = [\n qr/\x{20ac}/u,\n '\x{0} ];\n"] CUR = 55 LEN = 64 | With 5.33.7 it looks OK: |SV = PV(0x55dd1ca94ec0) at 0x55dd1c966400 REFCNT = 1 FLAGS = (POK,pPOK) PV = 0x55dd1c975960 "$VAR1 = [\n qr/\342\202\254/u,\n '\344'\n ];\n"\0 CUR = 55 LEN = 64 | Printing the dumped string gives me a � instead of a-umlaut. I suspect this could be due to #18619 <#18619> --- @arc <https://github.com/arc>, will you take a look?

I bisected this to c71f1f2 is the first bad commit commit c71f1f2 Author: Aaron Crane ***@***.***> Date: Wed Mar 3 18:11:45 2021 +0000 Make Data::Dumper mark regex output as UTF-8 if needed

[arc]

Sorry, I'm not sure when I'll have chance to look into this.

[khwilliamson]

I single stepped through and found where the fault occurs, but I don't know enough to know what the proper course of action should be.

In Dumper.xs, there is this code

   integer_came_from_string:
       c = SvPV(val, i);
       /* the pure perl and XS non-qq outputs have historically been
        * different in this case, but for useqq, let's try to match
        * the pure perl code.
        * see [perl #74798]
        */
       if (style->useqq && safe_decimal_number(c, i)) {
           sv_catsv(retval, val);
       }
       else if (DO_UTF8(val) || style->useqq)
           i += esc_q_utf8(aTHX_ retval, c, i, DO_UTF8(val), style->useqq);
       else {
           sv_grow(retval, SvCUR(retval)+3+2*i); /* 3: ""\0 */
           r = SvPVX(retval) + SvCUR(retval);
           r[0] = '\'';
           i += esc_q(r+1, c, i);
           ++i;
           r[i++] = '\'';
           r[i] = '\0';
           SvCUR_set(retval, SvCUR(retval)+i);
       }

What is happening is that val contains "\344" which c is set to, and in the final else that is concatenated to retval which has previously been upgraded to UTF-8. Thus we concatenate an illegal byte sequence to it.

[arc]

What happens if we add || SvUTF8(retval) to the else if condition?

Alternatively, should we change dump_regexp() to esc_q_utf8() on sv_pattern if it's SvUTF8?

[khwilliamson]

On 5/4/21 3:58 AM, Aaron Crane wrote: What happens if we add ||| SvUTF8(retval)| to the |else if| condition? Alternatively, should we change |dump_regexp()| to |esc_q_utf8()| on |sv_pattern| if it's SvUTF8?

The first thing doesn't work, and the 2nd I don't understand enough to try, but I think it's barking up the wrong tree. The problem here, from reading the man page, is that the user is giving a non-ASCII single character name to a variable, so it doesn't print as the default VAR1, etc. Your first suggestion still is about retval, and not about 'val', which is the name. (Better naming of variables would have helped here, unless this is a more general routine, of which this is a particular usage.) The name looks to be appended to retval, so both must be in UTF-8 or both not. I would think that the name should be displayed as-is, without being turned into a \x{...} output. I think that what should be done is for someone who understands the intent of the program to look at this. I wonder if the original author(s) even contemplated the possibility of a non-ASCII name.

[arc]

I don't think this is about names of dumped objects — the output in the original bug report (correctly AFAICT) uses $VAR1.

(Having looked, I think there's a separate bug in supplying supra-ASCII names for dumped objects, but I don't think that's what's going on here.)

The original report says that the 5.33.7 output looks good, but I'm unconvinced: the U+20AC is dumped as the three bytes of its UTF-8 representation, and the U+00E4 is dumped as the single byte of its Latin-1 representation. The bug has existed throughout; it's just that now its symptom is a "malformed UTF-8" error rather than a more silent "surprise mojibake" error.

Possibly the change in the bisected-to commit should be adjusted to quote the literal parts of supra-ASCII regexes using backslash notation (just as happens for strings). That might be an easier fix than ensuring everything else knows that the generated output might be SvUTF8.

[arc]

I've opened PR #18771 to address this issue.

I can't guarantee I'll be able to get that merged (once blead reopens), so if someone else could do that, I'd appreciate it greatly.

[nwc10]

Proposed fix in #18793