Steps 1 and 3 of PyUnicode_FromFormatV() doesn't handle the format string "%%" correctly. The loop responsible to skip the precision moves outside the format string, and the function will then read uninitialized memory. The loop:

             while (*++f && *f != '%' && !Py_ISALPHA((unsigned)*f))
                 ;

This is another issue:

    for (f = format; *f; f++) {
         if (*f == '%') {
             if (*(f+1)=='%')
                 continue;
    ...

continue only skips the first %: with "%%", the second % will be interpreted (and not escaped).

Attached patch fixes the issue, but I don't feal confortable with this ugly function, and I would appreciate a review :-) The patch adds unit tests.

I found the bug when trying to add new tests before trying to implement "%zi" format. I was first surprised that "%zi" (and %li and %lli) is not supported, but now I am surprised because I found bugs :-)

Oh, my first patch was wrong: it failed on %c format (in zipimport with "%U%c%U"). The second version of the patch should be better.

PyBytes_FromFormatV() has the same issue.

> PyBytes_FromFormatV() has the same issue.

Oh no, it's ok, the function works as expected.

Hi, haypo, Your patch seems cannot be applied cleanly on current py3k trunk. And after modified your patch, test_unicode.py runs into Segmentation fault. Is there something wrong or some changes which could influence this bug had been already made since the patch is worked out?


On the current trunk, I guess the bug could be fixed in a simpler way:

In step 1, before check '%%', check '%'(a string ends with '%') first. Then check '%%' and skip it.

The whole patch:

Index: Objects/unicodeobject.c
===================================================================
--- Objects/unicodeobject.c	(revision 88453)
+++ Objects/unicodeobject.c	(working copy)
@@ -750,8 +750,12 @@
      * result in an array) */
     for (f = format; *f; f++) {
          if (*f == '%') {
-             if (*(f+1)=='%')
+             if (*(f+1)=='\0')
+                continue;
+             if (*(f+1)=='%') {
+                 f++;
                  continue;
+             }
              if (*(f+1)=='S' || *(f+1)=='R' || *(f+1)=='A')
                  ++callcount;
              while (Py_ISDIGIT((unsigned)*f))


After applying this small patch and tests in your patch, test_unicode.py can passed.

Well, the main problem is that there are 3 different codes to parse the format string, and each code is different... Attached patch factorizes the code: create one subfunction parse_format_flags(). It fixes also this issue and prepares the work to fix #10831.

> Well, the main problem is that there are 3 different codes to parse the format string, and each code is different... Attached patch factorizes the code: create one subfunction parse_format_flags(). It fixes also this issue and prepares the work to fix #10831.


Sounds nice! Maybe several related issues can also use this, like #7330.

Fixed in Python 3.3 (r88702+r88703).

I don't really want to backport it to 3.2, 3.1 or 2.7, because the patch is complex, changes a very important function, and the fixed bug (don't crash on invalid format strings) is minor. If you would like the fix into in old release, please reopen the issue and tell me why.

Hi, haypo, would you mind modify your newly added parse_format_flags() function so that it can diff the precision value of '%.0s' and '%s'(Currently both of them return precision as 0)? Because if used with string formatters(%s, %R, %S, %A, ...), they should be very different. And so I can work on issue7330 with it.

Just return precision as -1 to indicate no precision is designated could work.

> Hi, haypo, would you mind modify your newly added parse_format_flags()
> function so that it can diff the precision value of '%.0s' and
> '%s'(Currently both of them return precision as 0)?

You should update your patch attached to #7330. I hope that your patch will be simpler with parse_format_flags() than before.