[cairo] [PATCH] Enable links to image files in SVG backend
Chris Wilson
chris at chris-wilson.co.uk
Wed Jan 20 03:25:04 PST 2010
Comments inline.
On Wed, 20 Jan 2010 12:56:44 +0200, Alexander Shulgin <alex.shulgin at gmail.com> wrote:
> OK, now I've got this:
>
> ---
> doc/public/cairo-sections.txt | 2 +
> doc/public/tmpl/cairo-surface.sgml | 14 +++
> src/Makefile.am | 2 +-
> src/cairo-surface.c | 17 +++-
> src/cairo-svg-surface.c | 136 ++++++++++++++++++++++++++-
> src/cairo.h | 2 +
> src/generate-uri-allowed-set-lookup-table.c | 27 ++++++
> src/uri-allowed-set-lookup-table.inc | 35 +++++++
> 8 files changed, 229 insertions(+), 6 deletions(-)
> create mode 100644 src/generate-uri-allowed-set-lookup-table.c
> create mode 100644 src/uri-allowed-set-lookup-table.inc
>
> diff --git a/doc/public/cairo-sections.txt b/doc/public/cairo-sections.txt
> index ff89db7..1e45c59 100644
> --- a/doc/public/cairo-sections.txt
> +++ b/doc/public/cairo-sections.txt
> @@ -176,6 +176,8 @@ cairo_svg_version_to_string
> CAIRO_MIME_TYPE_JP2
> CAIRO_MIME_TYPE_JPEG
> CAIRO_MIME_TYPE_PNG
> +CAIRO_MIME_TYPE_XURI
> +CAIRO_MIME_TYPE_XURI_ENCODED
I leave others to debate the merits of accepting a non-rfc3986 encoded
URI, and whether it is wise for Cairo just to mandate use of
g_filename_to_uri (or whatever) for local files.
> +/**
> + * _cairo_svg_surface_uri_encode:
> + *
> + * Percent-encode the URI string. Percent sign itself is not in
> + * allowed set so it gets encoded.
> + *
> + * RFC: http://tools.ietf.org/html/rfc3986
> + **/
This is not svg specific, so if we keep the uri encoding, we should create
a cairo-uri.c.
> +static cairo_status_t
> +_cairo_svg_surface_uri_encode (const unsigned char *uri, unsigned int length,
> + unsigned char **enc, unsigned int *enc_len)
> +{
> + unsigned char *p;
> + int i;
> +
> + *enc = malloc (length*3 + 1 /* to fit nul-byte from last snprintf */);
We need to do pessimistic integer overflow checks here. So use
_cairo_malloc_ab_plus_c(length, 3, 1); it's a pain but better than a
security exploit.
> + if (!*enc)
> + return CAIRO_STATUS_NO_MEMORY;
Please use return _cairo_error (CAIRO_STATUS_NO_MEMORY);
The _cairo_error() gives us a breakpoint on the origin of an error,
absolutely invaluable when tracking down library bugs.
> +
> + p = *enc;
> + for (i = 0; i < length; i++)
> + if (!uri_allowed_set[uri[i]]) {
> + snprintf (p, 4 /* "%XX\0" */, "%%%2X", uri[i]);
> + p += 3;
> + } else {
> + *(p++) = uri[i];
> + }
> +
> + *enc_len = p - *enc;
> +
> + return CAIRO_STATUS_SUCCESS;
> +}
> +
> static cairo_status_t
> _cairo_svg_surface_emit_surface (cairo_svg_document_t *document,
> cairo_surface_t *surface)
> @@ -1175,6 +1277,8 @@ _cairo_svg_surface_emit_surface
> (cairo_svg_document_t *document,
> cairo_rectangle_int_t extents;
> cairo_bool_t is_bounded;
> cairo_status_t status;
> + const unsigned char *uri;
> + unsigned int uri_len;
>
> if (_cairo_user_data_array_get_data (&surface->user_data,
> (cairo_user_data_key_t *) document))
> @@ -1192,10 +1296,34 @@ _cairo_svg_surface_emit_surface
> (cairo_svg_document_t *document,
>
> _cairo_output_stream_printf (document->xml_node_defs, " xlink:href=\"");
>
> - status = _cairo_surface_base64_encode (surface,
> - document->xml_node_defs);
> - if (unlikely (status))
> - return status;
> + cairo_surface_get_mime_data (surface, CAIRO_MIME_TYPE_XURI_ENCODED,
> + &uri, &uri_len);
> + if (uri)
> + _cairo_svg_surface_emit_attr_value (document->xml_node_defs,
> + uri, uri_len);
Use if (uri != NULL)
> + else {
No dangling blocks, so } else {
> + cairo_surface_get_mime_data (surface, CAIRO_MIME_TYPE_XURI,
> + &uri, &uri_len);
> + if (uri) {
> + unsigned char *enc;
> + unsigned int enc_len;
> +
> + status = _cairo_svg_surface_uri_encode (uri, uri_len,
> + &enc, &enc_len);
> + if (unlikely (status))
> + return status;
> +
> + _cairo_svg_surface_emit_attr_value (document->xml_node_defs,
> + enc, enc_len);
> +
> + free (enc);
> + } else {
> + status = _cairo_surface_base64_encode (surface,
> + document->xml_node_defs);
> + if (unlikely (status))
> + return status;
> + }
> + }
>
> _cairo_output_stream_printf (document->xml_node_defs, "\"/>\n");
>
> diff --git a/src/cairo.h b/src/cairo.h
> index 8be8043..16cafaf 100644
> --- a/src/cairo.h
> +++ b/src/cairo.h
> @@ -2042,6 +2042,8 @@ cairo_surface_set_user_data (cairo_surface_t *surface,
> #define CAIRO_MIME_TYPE_JPEG "image/jpeg"
> #define CAIRO_MIME_TYPE_PNG "image/png"
> #define CAIRO_MIME_TYPE_JP2 "image/jp2"
> +#define CAIRO_MIME_TYPE_XURI "text/x-uri"
> +#define CAIRO_MIME_TYPE_XURI_ENCODED "text/x-uri-encoded"
If we were to accepted both XURI and XURI_ENCODED, the one we want people
to use most often should have the shorter name, ie XURI and
XURI_NON_ENCODED. What's the justification for the non-encoded XURI
again? ;-)
> cairo_public void
> cairo_surface_get_mime_data (cairo_surface_t *surface,
> diff --git a/src/generate-uri-allowed-set-lookup-table.c
> b/src/generate-uri-allowed-set-lookup-table.c
> new file mode 100644
> index 0000000..702a973
> --- /dev/null
> +++ b/src/generate-uri-allowed-set-lookup-table.c
> @@ -0,0 +1,27 @@
> +/* vim: set sw=4 sts=4: -*- Mode: c; tab-width: 8; c-basic-offset: 4;
> indent-tabs-mode: t; -*- */
> +#include <stdio.h>
> +#include <string.h>
> +#include <ctype.h>
> +
> +static const char *uri_allowed_set =
> "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-._~:/?#[]@!$&'()*+,;=";
> +
> +int
> +main ()
> +{
> + int i;
> +
> + printf ("/* %s */\n", uri_allowed_set);
> + printf ("static const char uri_allowed_set[256] = {\n 0,");
> + for (i = 1; i < 256; i++) {
> + printf (strchr (uri_allowed_set, (char) i) ? " 1" : " 0");
> +
> + if (isgraph (i))
> + printf (" /*%c*/", i);
> +
> + putchar(',');
> + if (i % 8 == 7)
> + putchar('\n');
> + }
> + printf ("};\n");
> + return 0;
> +}
> diff --git a/src/uri-allowed-set-lookup-table.inc
> b/src/uri-allowed-set-lookup-table.inc
I'm resistant to the first .inc file, it's not a convention in wide usage.
In this case, I would just include it directly in cairo-uri.c with the
generator in a comment (which would be even more useful if was directly
executable, for example a python script ;-).
> new file mode 100644
> index 0000000..a6caa9d
> --- /dev/null
> +++ b/src/uri-allowed-set-lookup-table.inc
> @@ -0,0 +1,35 @@
> +/* ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-._~:/?#[]@!$&'()*+,;=
> */
> +static const char uri_allowed_set[256] = {
> + 0, 0, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0, 0, 0, 0, 0, 0,
> + 0, 1 /*!*/, 0 /*"*/, 1 /*#*/, 1 /*$*/, 0 /*%*/, 1 /*&*/, 1 /*'*/,
> + 1 /*(*/, 1 /*)*/, 1 /***/, 1 /*+*/, 1 /*,*/, 1 /*-*/, 1 /*.*/, 1 /*/*/,
> + 1 /*0*/, 1 /*1*/, 1 /*2*/, 1 /*3*/, 1 /*4*/, 1 /*5*/, 1 /*6*/, 1 /*7*/,
> + 1 /*8*/, 1 /*9*/, 1 /*:*/, 1 /*;*/, 0 /*<*/, 1 /*=*/, 0 /*>*/, 1 /*?*/,
> + 1 /*@*/, 1 /*A*/, 1 /*B*/, 1 /*C*/, 1 /*D*/, 1 /*E*/, 1 /*F*/, 1 /*G*/,
> + 1 /*H*/, 1 /*I*/, 1 /*J*/, 1 /*K*/, 1 /*L*/, 1 /*M*/, 1 /*N*/, 1 /*O*/,
> + 1 /*P*/, 1 /*Q*/, 1 /*R*/, 1 /*S*/, 1 /*T*/, 1 /*U*/, 1 /*V*/, 1 /*W*/,
> + 1 /*X*/, 1 /*Y*/, 1 /*Z*/, 1 /*[*/, 0 /*\*/, 1 /*]*/, 0 /*^*/, 1 /*_*/,
> + 0 /*`*/, 1 /*a*/, 1 /*b*/, 1 /*c*/, 1 /*d*/, 1 /*e*/, 1 /*f*/, 1 /*g*/,
> + 1 /*h*/, 1 /*i*/, 1 /*j*/, 1 /*k*/, 1 /*l*/, 1 /*m*/, 1 /*n*/, 1 /*o*/,
> + 1 /*p*/, 1 /*q*/, 1 /*r*/, 1 /*s*/, 1 /*t*/, 1 /*u*/, 1 /*v*/, 1 /*w*/,
> + 1 /*x*/, 1 /*y*/, 1 /*z*/, 0 /*{*/, 0 /*|*/, 0 /*}*/, 1 /*~*/, 0,
> + 0, 0, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0, 0, 0, 0, 0, 0,
> +};
> --
> 1.6.3.3
>
> Is this looking any good? :)
Yes. :-)
-ickle
--
Chris Wilson, Intel Open Source Technology Centre
More information about the cairo
mailing list