From 52bf4bb6ee4444110cd22f8ec2ddd2edec7d0d08 Mon Sep 17 00:00:00 2001 From: Alx Sa Date: Tue, 25 Nov 2025 23:27:52 +0000 Subject: [PATCH] path: Update path import code from C-based librsvg GIMP's path import code is largely copied from an earlier version of rsvg. Therefore, it does not include updates that made number processing more robust (such as properly parsing 'c1.5.2' as '1.5, 0.2' for curve coordinates) Per the suggestion by @federico, this patch updates our code using rsvg-path.c from the last version of the C-based librsvg (updated November 3rd, 2016). Our original code also broke if there was any spaces after the last item in the SVG transform attribute. This patch adds a call to g_strstrip () to trim any unnecessary spaces either before or after the transform string to prevent this issue. --- app/path/gimppath-import.c | 384 ++++++++++++++++++++++--------------- 1 file changed, 233 insertions(+), 151 deletions(-) diff --git a/app/path/gimppath-import.c b/app/path/gimppath-import.c index 29fee22931..202c2d9ab5 100644 --- a/app/path/gimppath-import.c +++ b/app/path/gimppath-import.c @@ -67,6 +67,8 @@ .direction = 0.0 \ } +#define GOT_SIGN 0x1 +#define GOT_EXPONENT_SIGN 0x2 typedef struct { @@ -105,6 +107,16 @@ typedef struct } SvgPath; +typedef enum +{ + IN_PREINTEGER, + IN_INTEGER, + IN_FRACTION, + IN_PREEXPONENT, + IN_EXPONENT +} SvgNumberInput; + + static gboolean gimp_path_import (GimpImage *image, GFile *file, const gchar *str, @@ -1212,11 +1224,14 @@ static gboolean parse_svg_transform (const gchar *value, GimpMatrix3 *matrix) { - gint i; + gint i; + gchar *safe_value; + + safe_value = g_strstrip (g_strdup (value)); gimp_matrix3_identity (matrix); - for (i = 0; value[i]; i++) + for (i = 0; safe_value[i]; i++) { GimpMatrix3 trafo; gchar keyword[32]; @@ -1227,31 +1242,31 @@ parse_svg_transform (const gchar *value, gimp_matrix3_identity (&trafo); /* skip initial whitespace */ - while (g_ascii_isspace (value[i])) + while (g_ascii_isspace (safe_value[i])) i++; /* parse keyword */ for (key_len = 0; key_len < sizeof (keyword); key_len++) { - gchar c = value[i]; + gchar c = safe_value[i]; if (g_ascii_isalpha (c) || c == '-') - keyword[key_len] = value[i++]; + keyword[key_len] = safe_value[i++]; else break; } if (key_len >= sizeof (keyword)) - return FALSE; + goto out; keyword[key_len] = '\0'; /* skip whitespace */ - while (g_ascii_isspace (value[i])) + while (g_ascii_isspace (safe_value[i])) i++; - if (value[i] != '(') - return FALSE; + if (safe_value[i] != '(') + goto out; i++; for (n_args = 0; ; n_args++) @@ -1260,29 +1275,33 @@ parse_svg_transform (const gchar *value, gchar *end_ptr; /* skip whitespace */ - while (g_ascii_isspace (value[i])) + while (g_ascii_isspace (safe_value[i])) i++; - c = value[i]; + c = safe_value[i]; if (g_ascii_isdigit (c) || c == '+' || c == '-' || c == '.') { if (n_args == G_N_ELEMENTS (args)) - return FALSE; /* too many args */ + goto out; /* too many args */ - args[n_args] = g_ascii_strtod (value + i, &end_ptr); - i = end_ptr - value; + args[n_args] = g_ascii_strtod (safe_value + i, &end_ptr); + i = end_ptr - safe_value; - while (g_ascii_isspace (value[i])) + while (g_ascii_isspace (safe_value[i])) i++; /* skip optional comma */ - if (value[i] == ',') + if (safe_value[i] == ',') i++; } else if (c == ')') - break; + { + break; + } else - return FALSE; + { + goto out; + } } /* OK, have parsed keyword and args, now calculate the transform matrix */ @@ -1290,7 +1309,7 @@ parse_svg_transform (const gchar *value, if (strcmp (keyword, "matrix") == 0) { if (n_args != 6) - return FALSE; + goto out; gimp_matrix3_affine (&trafo, args[0], args[1], @@ -1302,7 +1321,7 @@ parse_svg_transform (const gchar *value, if (n_args == 1) args[1] = 0.0; else if (n_args != 2) - return FALSE; + goto out; gimp_matrix3_translate (&trafo, args[0], args[1]); } @@ -1311,7 +1330,7 @@ parse_svg_transform (const gchar *value, if (n_args == 1) args[1] = args[0]; else if (n_args != 2) - return FALSE; + goto out; gimp_matrix3_scale (&trafo, args[0], args[1]); } @@ -1328,25 +1347,25 @@ parse_svg_transform (const gchar *value, gimp_matrix3_translate (&trafo, args[1], args[2]); } else - return FALSE; + goto out; } else if (strcmp (keyword, "skewX") == 0) { if (n_args != 1) - return FALSE; + goto out; gimp_matrix3_xshear (&trafo, tan (gimp_deg_to_rad (args[0]))); } else if (strcmp (keyword, "skewY") == 0) { if (n_args != 1) - return FALSE; + goto out; gimp_matrix3_yshear (&trafo, tan (gimp_deg_to_rad (args[0]))); } else { - return FALSE; /* unknown keyword */ + goto out; /* unknown keyword */ } gimp_matrix3_invert (&trafo); @@ -1356,6 +1375,10 @@ parse_svg_transform (const gchar *value, gimp_matrix3_invert (matrix); return TRUE; + +out: + g_free (safe_value); + return FALSE; } @@ -1383,6 +1406,13 @@ static void parse_path_default_xy (ParsePathContext *ctx, gint n_params); static void parse_path_do_cmd (ParsePathContext *ctx, gboolean final); +static gint parse_number (ParsePathContext *ctx, + const char *data); +static void path_end_of_number (ParsePathContext *ctx, + gdouble val, + gint sign, + gint exp_sign, + gint exp); static GList * @@ -1390,140 +1420,19 @@ parse_path_data (const gchar *data) { ParsePathContext ctx; - gboolean in_num = FALSE; - gboolean in_frac = FALSE; - gboolean in_exp = FALSE; - gboolean exp_wait_sign = FALSE; - gdouble val = 0.0; - gchar c = 0; - gint sign = 0; - gint exp = 0; - gint exp_sign = 0; - gdouble frac = 0.0; - gint i; + gchar c = 0; + gint i; memset (&ctx, 0, sizeof (ParsePathContext)); - for (i = 0; ; i++) + for (i = 0; data[i] != '\0'; i++) { c = data[i]; - if (c >= '0' && c <= '9') + if ((c >= '0' && c <= '9') || c == '+' || c == '-' || c == '.') { /* digit */ - if (in_num) - { - if (in_exp) - { - exp = (exp * 10) + c - '0'; - exp_wait_sign = FALSE; - } - else if (in_frac) - val += (frac *= 0.1) * (c - '0'); - else - val = (val * 10) + c - '0'; - } - else - { - in_num = TRUE; - in_frac = FALSE; - in_exp = FALSE; - exp = 0; - exp_sign = 1; - exp_wait_sign = FALSE; - val = c - '0'; - sign = 1; - } - } - else if (c == '.') - { - if (! in_num) - { - in_num = TRUE; - val = 0; - } - - in_frac = TRUE; - frac = 1; - } - else if ((c == 'E' || c == 'e') && in_num) - { - in_exp = TRUE; - exp_wait_sign = TRUE; - exp = 0; - exp_sign = 1; - } - else if ((c == '+' || c == '-') && in_exp) - { - exp_sign = c == '+' ? 1 : -1; - } - else if (in_num) - { - /* end of number */ - - val *= sign * pow (10, exp_sign * exp); - - if (ctx.rel) - { - /* Handle relative coordinates. This switch statement attempts - to determine _what_ the coords are relative to. This is - underspecified in the 12 Apr working draft. */ - switch (ctx.cmd) - { - case 'l': - case 'm': - case 'c': - case 's': - case 'q': - case 't': - /* rule: even-numbered params are x-relative, odd-numbered - are y-relative */ - if ((ctx.param & 1) == 0) - val += ctx.cpx; - else if ((ctx.param & 1) == 1) - val += ctx.cpy; - break; - - case 'a': - /* rule: sixth and seventh are x and y, rest are not - relative */ - if (ctx.param == 5) - val += ctx.cpx; - else if (ctx.param == 6) - val += ctx.cpy; - break; - - case 'h': - /* rule: x-relative */ - val += ctx.cpx; - break; - - case 'v': - /* rule: y-relative */ - val += ctx.cpy; - break; - } - } - - ctx.params[ctx.param++] = val; - parse_path_do_cmd (&ctx, FALSE); - in_num = FALSE; - } - - if (c == '\0') - { - break; - } - else if ((c == '+' || c == '-') && ! exp_wait_sign) - { - sign = c == '+' ? 1 : -1; - val = 0; - in_num = TRUE; - in_frac = FALSE; - in_exp = FALSE; - exp = 0; - exp_sign = 1; - exp_wait_sign = FALSE; + i += parse_number (&ctx, data + i) - 1; } else if (c == 'z' || c == 'Z') { @@ -1782,3 +1691,176 @@ parse_path_do_cmd (ParsePathContext *ctx, break; } } + +static gint +parse_number (ParsePathContext *ctx, + const char *data) +{ + gint length = 0; + /* Current location within the number */ + gint in = IN_PREINTEGER; + /* [bitfield] Having 2 of each of these is an error */ + gint got = 0x0; + /* Set to true if the number should end after a char */ + gboolean end = FALSE; + /* Set to true if the number ended due to an error */ + gboolean error = FALSE; + gdouble value = 0.0; + gdouble fraction = 1.0; + /* Presume the INTEGER is positive if it has no sign */ + gint sign = +1; + gint exponent = 0; + /* Presume the EXPONENT is positive if it has no sign */ + gint exponent_sign = +1; + + while (data[length] != '\0' && ! end && ! error) + { + gchar c = data[length]; + + switch (in) + { + case IN_PREINTEGER: /* No numbers yet, we're just starting out */ + /* LEGAL: + - .->FRACTION DIGIT->INTEGER */ + if (c == '+' || c == '-') + { + if (got & GOT_SIGN) + { + error = TRUE; /* Two signs: not allowed */ + } + else + { + sign = c == '+' ? +1 : -1; + got |= GOT_SIGN; + } + } + else if (c == '.') + { + in = IN_FRACTION; + } + else if (c >= '0' && c <= '9') + { + value = c - '0'; + in = IN_INTEGER; + } + break; + + case IN_INTEGER: /* Previous character(s) was/were digit(s) */ + /* LEGAL: DIGIT .->FRACTION E->PREEXPONENT */ + if (c >= '0' && c <= '9') + value = value * 10 + (c - '0'); + else if (c == '.') + in = IN_FRACTION; + else if (c == 'e' || c == 'E') + in = IN_PREEXPONENT; + else + end = TRUE; + break; + + case IN_FRACTION: /* Previously, digit(s) in the fractional part */ + /* LEGAL: DIGIT E->PREEXPONENT */ + if (c >= '0' && c <= '9') + { + fraction *= 0.1; + value += fraction * (c - '0'); + } + else if (c == 'e' || c == 'E') + { + in = IN_PREEXPONENT; + } + else + { + end = TRUE; + } + break; + + case IN_PREEXPONENT: /* Right after E */ + /* LEGAL: + - DIGIT->EXPONENT */ + if (c == '+' || c == '-') + { + if (got & GOT_EXPONENT_SIGN) + { + error = TRUE; /* Two signs: not allowed */ + } + else + { + exponent_sign = c == '+' ? +1 : -1; + got |= GOT_EXPONENT_SIGN; + } + } + else if (c >= '0' && c <= '9') + { + exponent = c - '0'; + in = IN_EXPONENT; + } + break; + + case IN_EXPONENT: /* After E and the sign, if applicable */ + /* LEGAL: DIGIT */ + if (c >= '0' && c <= '9') + exponent = exponent * 10 + (c - '0'); + else + end = TRUE; + break; + } + length++; + } + + path_end_of_number (ctx, value, sign, exponent_sign, exponent); + + return end ? length - 1 : length; +} + +static void +path_end_of_number (ParsePathContext *ctx, + gdouble val, + gint sign, + gint exp_sign, + gint exp) +{ + val *= sign * pow (10, exp_sign * exp); + + if (ctx->rel) + { + /* Handle relative coordinates. This switch statement attempts + to determine _what_ the coords are relative to. This is + underspecified in the 12 Apr working draft. */ + switch (ctx->cmd) + { + case 'l': + case 'm': + case 'c': + case 's': + case 'q': + case 't': + /* rule: even-numbered params are x-relative, odd-numbered + are y-relative */ + if ((ctx->param & 1) == 0) + val += ctx->cpx; + else if ((ctx->param & 1) == 1) + val += ctx->cpy; + break; + + case 'a': + /* rule: sixth and seventh are x and y, rest are not + relative */ + if (ctx->param == 5) + val += ctx->cpx; + else if (ctx->param == 6) + val += ctx->cpy; + break; + + case 'h': + /* rule: x-relative */ + val += ctx->cpx; + break; + + case 'v': + /* rule: y-relative */ + val += ctx->cpy; + break; + } + } + ctx->params[ctx->param++] = val; + + parse_path_do_cmd (ctx, FALSE); +}