diff --git a/pd/src/x_connective.c b/pd/src/x_connective.c
index 60afd520f6b49fd642d1c435272c09e470bece1e..7f92aec6a55b2c2161c0ba787dc38d07d7a947a4 100644
--- a/pd/src/x_connective.c
+++ b/pd/src/x_connective.c
@@ -1464,45 +1464,165 @@ typedef struct _makefilename
t_printtype x_accept;
} t_makefilename;
-static const char* makefilename_doscanformat(const char *str, t_printtype *typ)
+static const char* makefilename_doscanformat(const char *str, t_printtype *typ,
+ char *errormsg)
{
- int infmt=0;
+ int infmt = 0, i;
for (; *str; str++)
{
- if (!infmt && *str=='%')
+ if (!infmt && *str == '%')
{
- infmt=1;
+ /* A string consisting of a single '%' character isn't a valid
+ format specifier. */
+ if (*(str+1) == '\0')
+ {
+ str++;
+ sprintf(errormsg, "field type missing after '%%'");
+ *typ = NONE;
+ return str;
+ }
+ infmt = 1;
continue;
}
if (infmt)
{
- if (*str=='%')
+ /* 1) flag field
+ Check for a leading '%' after our format specifier. This
+ will be converted to a single '%' in the output. */
+ if (*str == '%')
{
- infmt=0;
+ /* Not in a format specifier after all, so start over... */
+ infmt = 0;
continue;
}
- if (strchr("-.#0123456789",*str)!=0)
- continue;
- if (*str=='s')
+ for (i = 0; *str && strchr("-+#0", *str) != 0; str++)
+ {
+ /* Check for flag chars. While a space is a legal part of this
+ field, Pd's parser would split it off into a separate arg.
+ Since makefilename has always truncated extra args we don't
+ support spaces here. */
+ /* Since we're dealing with arbitrary input let's keep
+ the total number of flags sane. */
+ if (i > 16)
+ {
+ sprintf(errormsg, "too many flags");
+ *typ = NONE;
+ return str;
+ }
+ }
+ /* 2) width field
+ Consecutive digits. Technically a width field may also be
+ '*' to use a variable to set the value. But makefilename has
+ never supported that, either generating a memory error or crash
+ upon use. So we exclude it here. */
+ if (*str == '*')
+ {
+ sprintf(errormsg, "variable width value not supported");
+ *typ = NONE;
+ return str;
+ }
+ int maxwidth = 3;
+ for (i = 0; *str && strchr("0123456789", *str) != 0; str++, i++)
+ {
+ /* Limit width length to prevent out-of-memory errors. */
+ if (i >= maxwidth)
+ {
+ sprintf(errormsg, "width field cannot be greater than "
+ "%d digits", maxwidth);
+ *typ = NONE;
+ return str;
+ }
+ }
+ /* 3) precision field
+ A '.' followed by consecutive digits. Can also have a '*' for
+ variable, so we need to check and exclude as with the width
+ field above. */
+ if (*str == '.')
+ {
+ str++;
+ if (*str == '*')
+ {
+ sprintf(errormsg, "variable precision field not supported");
+ *typ = NONE;
+ return str;
+ }
+ int maxwidth = 3;
+ for (i = 0; *str && strchr("0123456789", *str) != 0; str++, i++)
+ {
+ if (i >= maxwidth)
+ {
+ sprintf(errormsg, "precision field cannot be greater "
+ "than %d digits", maxwidth);
+ *typ = NONE;
+ return str;
+ }
+ }
+ /* Fairly certain having '.' with no trailing digits or asterisk
+ is undefined behavior. So let's trigger a syntax error. */
+ if (i == 0)
+ {
+ sprintf(errormsg, "precision specifier requires a number "
+ "following the '.'");
+ *typ = NONE;
+ return str;
+ }
+ }
+ /* 4) length field
+ Fairly certain the length field doesn't make any sense
+ here. At least I've never seen it used, so let's skip it. */
+
+ /* 5) type field
+ The type of value we want to fill the slot with. This is the
+ most complex field we support. */
+
+ /* a C string */
+ if (*str == 's')
{
*typ = STRING;
return str;
}
- if (strchr("fgGeE",*str)!=0)
+
+ /* a float. There's also 'a' and 'A' from C99, but let's stick
+ to the old school ones for now... */
+ if (*str && strchr("fFgGeE", *str) != 0)
{
*typ = FLOAT;
return str;
}
- if (strchr("xXdiouc",*str)!=0)
+
+ /* an int */
+ if (*str && strchr("xXdiouc", *str) != 0)
{
*typ = INT;
return str;
}
- if (strchr("p",*str)!=0)
+
+ /* a pointer */
+ if (*str && strchr("p", *str) != 0)
{
*typ = POINTER;
return str;
}
+
+ /* if we've gotten here it means we are missing a type field.
+ Undefined behavior would result, so we will bail here */
+
+ /* First, let's check for any remaining type fields which
+ we don't support. That way we can give a better error message. */
+ if (*str == 'a' || *str == 'A')
+ sprintf(errormsg, "hexfloat type not supported. If you "
+ "need this feature make a case on the "
+ "mailing list and we'll consider adding it");
+ else if (*str == 'n')
+ sprintf(errormsg, "n field type not supported");
+ else if (*str == 'm')
+ sprintf(errormsg, "m field type not supported");
+ else if (*str)
+ sprintf(errormsg, "bad field type '%c'", *str);
+ else
+ sprintf(errormsg, "field type missing");
+ *typ = NONE;
+ return str;
}
}
*typ = NONE;
@@ -1511,21 +1631,41 @@ static const char* makefilename_doscanformat(const char *str, t_printtype *typ)
static void makefilename_scanformat(t_makefilename *x)
{
+ char errorbuf[MAXPDSTRING];
+ errorbuf[0] = '\0';
const char *str;
t_printtype typ;
if (!x->x_format) return;
str = x->x_format->s_name;
- str = makefilename_doscanformat(str, &typ);
+ /* First attempt at parsing the format string for the field type. */
+ str = makefilename_doscanformat(str, &typ, errorbuf);
+ /* If we got any errors we will have some content in our errorbuf... */
+ if (*errorbuf)
+ {
+ pd_error(x, "makefilename: invalid format string '%s' "
+ "(%s). Setting to empty string.",
+ x->x_format->s_name, errorbuf);
+ /* set the format string to nothing. It would be great to refuse to
+ create the object here, but we also have to deal with new format
+ strings with the 'set' message. So for consistency we zero out
+ the format string and make it a runtimer error. */
+ x->x_format = 0;
+ return;
+ }
x->x_accept = typ;
- if (str && (NONE != typ))
+ if (str && (typ != NONE))
{
- /* try again, to see if there's another format specifier
- (which we forbid) */
- str = makefilename_doscanformat(str, &typ);
- if (NONE != typ)
+ /* try again, to see if there's another format specifier
+ (which we forbid) */
+ str = makefilename_doscanformat(str, &typ, errorbuf);
+ /* If we've got a type other than none-- OR if we've got something
+ in our errorbuf-- we've got a syntax error. */
+ if (typ != NONE || *errorbuf)
{
pd_error(x, "makefilename: invalid format string '%s' "
- "(too many format specifiers)", x->x_format->s_name);
+ "(too many format specifiers). "
+ "Setting to empty string.",
+ x->x_format->s_name);
x->x_format = 0;
return;
}
@@ -1572,7 +1712,7 @@ static void makefilename_float(t_makefilename *x, t_floatarg f)
switch(x->x_accept)
{
case NONE:
- makefilename_snprintf(x, buf, "%s", x->x_format->s_name);
+ makefilename_snprintf(x, buf, x->x_format->s_name);
break;
case INT: case POINTER:
makefilename_snprintf(x, buf, x->x_format->s_name, (int)f);
@@ -1613,7 +1753,7 @@ static void makefilename_symbol(t_makefilename *x, t_symbol *s)
makefilename_snprintf(x, buf, x->x_format->s_name, 0.);
break;
case NONE:
- makefilename_snprintf(x, buf, "%s", x->x_format->s_name);
+ makefilename_snprintf(x, buf, x->x_format->s_name);
break;
default:
makefilename_snprintf(x, buf, "%s", x->x_format->s_name);
@@ -1639,7 +1779,7 @@ static void makefilename_bang(t_makefilename *x)
makefilename_snprintf(x, buf, x->x_format->s_name, 0.);
break;
case NONE:
- makefilename_snprintf(x, buf, "%s", x->x_format->s_name);
+ makefilename_snprintf(x, buf, x->x_format->s_name);
break;
default:
makefilename_snprintf(x, buf, "%s", x->x_format->s_name);
diff --git a/scripts/regression_tests.pd b/scripts/regression_tests.pd
index fcdd0847a1c5ab6a2115fc7d03631a915c62d17e..6b7af932edfb94f49b3c6f47709cdb7a20d3cb9d 100644
--- a/scripts/regression_tests.pd
+++ b/scripts/regression_tests.pd
@@ -1,29 +1,29 @@
#N canvas 3 60 749 617 12;
-#X obj 345 301 r \$0-result;
-#X obj 345 326 route 0;
-#X obj 453 470 print failure;
-#X obj 430 336 tgl 28 0 empty empty Print_All_Results 31 11 0 12 -262144
--1 -1 1 1;
+#X obj 475 301 r \$0-result;
+#X obj 475 326 route 0;
+#X obj 583 470 print failure;
+#X obj 560 336 tgl 28 0 empty empty Print_All_Results 31 11 0 12 -262144
+-1 -1 0 1;
#X obj 159 149 bng 31 250 50 0 empty empty Run_all 39 13 0 12 -262144
-1 -1;
#X obj 56 25 r init;
#X obj 345 191 route dollarzero;
-#X obj 345 411 t b a;
-#X obj 345 541 s pd;
+#X obj 475 411 t b a;
+#X obj 475 541 s pd;
#X obj 56 120 trigger bang bang anything;
#X msg 56 145 gui;
#X obj 56 170 pdinfo;
#X obj 56 195 sel 0;
#X obj 56 245 s pd;
-#X msg 345 516 quit 1;
+#X msg 475 516 quit 1;
#X msg 56 220 quit;
#X obj 145 191 rtest msg_dollarzero;
#X obj 145 246 rtest msg_dollarzero_semi;
#X obj 145 302 rtest msg_click;
#X obj 345 216 rtest binbuf_dollarzero;
-#X msg 345 440 gui;
-#X obj 345 465 pdinfo;
-#X obj 345 490 sel 0;
+#X msg 475 440 gui;
+#X obj 475 465 pdinfo;
+#X obj 475 490 sel 0;
#X text 117 25 <- we start Pd with the -send "init etc." flag. This
will automatically start the tests and allow us to send a comma-separated
list of messages which will be evaluated by Pd without a target. This
@@ -34,9 +34,10 @@ is handy for some binbuf tests.;
#X text 536 150 <- we have to escape the arg;
#X text 556 190 escape it in a comment.;
#X text 556 170 in bash but we can't;
-#X obj 391 374 spigot;
-#X obj 407 440 route 1;
-#X obj 407 495 print success;
+#X obj 521 374 spigot;
+#X obj 537 440 route 1;
+#X obj 537 495 print success;
+#X obj 145 353 rtest makefilename_percent_parsing;
#X connect 0 0 1 0;
#X connect 1 0 7 0;
#X connect 1 1 29 0;
@@ -56,6 +57,7 @@ is handy for some binbuf tests.;
#X connect 15 0 13 0;
#X connect 16 0 17 0;
#X connect 17 0 18 0;
+#X connect 18 0 32 0;
#X connect 20 0 21 0;
#X connect 21 0 22 0;
#X connect 22 0 14 0;
diff --git a/scripts/regression_tests/makefilename_percent_parsing.pd b/scripts/regression_tests/makefilename_percent_parsing.pd
new file mode 100644
index 0000000000000000000000000000000000000000..a162f073c0a830748e4ea3b8c4d243bfde86725d
--- /dev/null
+++ b/scripts/regression_tests/makefilename_percent_parsing.pd
@@ -0,0 +1,18 @@
+#N canvas 430 102 538 300 12;
+#X obj 41 28 inlet;
+#X msg 41 53 1;
+#X obj 41 78 makefilename %%s%g;
+#X obj 41 103 select %s1;
+#X obj 108 133 list prepend 0;
+#X msg 41 133 1 %s1;
+#X obj 41 209 outlet;
+#X obj 41 168 list append format specifier '%%' should get converted
+to a single '%'. Expected output: '%s1'. Actual output: \$2;
+#X connect 0 0 1 0;
+#X connect 1 0 2 0;
+#X connect 2 0 3 0;
+#X connect 3 0 5 0;
+#X connect 3 1 4 0;
+#X connect 4 0 7 0;
+#X connect 5 0 7 0;
+#X connect 7 0 6 0;