sprintf(): handle mangled formats better with utf8

Currently if sprintf() detects an error in the format while processing
a %.... entry, it copies the bytes as-is from the % to the point the
error was detected, then continues, If the output string and format string
don't have the same utf8-ness, this can result in badly-formed utf8
output.

This commit changes the code so that it just appends a '%' then restarts
processing from the character following the %. Most of the time this just
again results with the characters following the % being output as-is,
expect this time the 'normal' character-copying code path is taken, which
handles utf8 mismatches correctly.

By doing this, it also removes a block of code which contained a "roll
your own" string appender which used SvGROW() and Copy(). This was one
further place which was potentially open to wrapping and block overrun
bugs.

This commit may cause occasional changes in behaviour, depending on
whether there are any further '%' characters within the bad section of the
format.  Now these will be reprocessed, possibly triggering further
'Invalid conversion' type warnings.

diff --git a/sv.c b/sv.c
index 285b7a6..2490b15 100644 (file)
--- a/sv.c
+++ b/sv.c
@@ -11444,6 +11444,12 @@ S_hextract(pTHX_ const NV nv, int* exponent, bool *subnormal,
     } STMT_END
 
 void
+
+
+/* This function assumes that pat has the same utf8-ness as sv.
+ * It's the caller's responsibility to ensure that this is so.
+ */
+
 Perl_sv_vcatpvfn_flags(pTHX_ SV *const sv, const char *const pat, const STRLEN patlen,
                        va_list *const args, SV **const svargs, const I32 svmax, bool *const maybe_tainted,
                        const U32 flags)
@@ -13003,19 +13009,10 @@ Perl_sv_vcatpvfn_flags(pTHX_ SV *const sv, const char *const pat, const STRLEN p
                Perl_warner(aTHX_ packWARN(WARN_PRINTF), "%" SVf, SVfARG(msg)); /* yes, this is reentrant */
            }
 
-           /* output mangled stuff ... */
-           if (c == '\0')
-               --q;
-           eptr = p;
-           elen = q - p;
-
-           /* ... right here, because formatting flags should not apply */
-           SvGROW(sv, SvCUR(sv) + elen + 1);
-           p = SvEND(sv);
-           Copy(eptr, p, elen, char);
-           p += elen;
-           *p = '\0';
-           SvCUR_set(sv, p - SvPVX_const(sv));
+           /* mangled format: output the '%', then continue from the
+             * character following that */
+            sv_catpvn_nomg(sv, p, 1);
+            q = p + 1;
            svix = osvix;
            continue;   /* not "break" */
        }

diff --git a/t/op/sprintf.t b/t/op/sprintf.t
index 18bee69..551f5ac 100644 (file)
--- a/t/op/sprintf.t
+++ b/t/op/sprintf.t
@@ -689,7 +689,7 @@ __END__
 >%*2$1d<       >[12, 3]<       >%*2$1d INVALID REDUNDANT<
 >%0v2.2d<      >''<    ><
 >%vc,%d<       >[63, 64, 65]<  >%vc,63 INVALID REDUNDANT<
->%v%,%d<       >[63, 64, 65]<  >%v%,63 INVALID REDUNDANT<
+>%v%,%d<       >[63, 64, 65]<  >%v%,63 INVALID INVALID REDUNDANT<
 >%vd,%d<       >["\x1", 2, 3]< >1,2 REDUNDANT<
 >%vf,%d<       >[1, 2, 3]<     >%vf,1 INVALID REDUNDANT<
 >%vF,%d<       >[1, 2, 3]<     >%vF,1 INVALID REDUNDANT<

diff --git a/t/op/sprintf2.t b/t/op/sprintf2.t
index c2308bb..7328b8f 100644 (file)
--- a/t/op/sprintf2.t
+++ b/t/op/sprintf2.t
@@ -939,5 +939,29 @@ SKIP: {
     }
 }
 
+{
+    # handle utf8 correctly when skipping invalid format
+    my $w_red   = 0;
+    my $w_inv   = 0;
+    my $w_other = 0;
+    local $SIG{__WARN__} = sub {
+        if ($_[0] =~ /^Invalid conversion/) {
+            $w_inv++;
+        }
+        elsif ($_[0] =~ /^Redundant argument/) {
+            $w_red++;
+        }
+        else {
+            $w_other++;
+        }
+    };
+
+    use warnings;
+    my $s = sprintf "%s%\xc4\x80%s", "\x{102}", "\xc4\x83";
+    is($s, "\x{102}%\xc4\x80\xc4\x83", "utf8 for invalid format");
+    is($w_inv,   1, "utf8 for invalid format: invalid warnings");
+    is($w_red,   0, "utf8 for invalid format: redundant warnings");
+    is($w_other, 0, "utf8 for invalid format: other warnings");
+}
 
 done_testing();