[cairo] [PATCH] Enable links to image files in SVG backend

[Alexander Shulgin]

On Wed, Jan 20, 2010 at 13:25, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> 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:
>>
>> 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.

I don't think forcing clients depend on glib or any other third-party
lib for this is in any way friendly.  And yet again, I've come to this
problem from Ruby, so this might cause even more trouble for clients.

>> +/**
>> + * _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.

Agreed, so lets decide whether we keep this first.

>> +    *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.

OK, no problem.

>> +    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.

Oh, this is interesting.  Never thought something like this is
possible, but it's so simple. :)

>> 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? ;-)

The more I think about it, the more I'm in favor of renaming these to
XFILENAME and XURI (or XURI_ENCODED if you want it to be harder to
type (; ).

Thing is, producing valid URIs from filenames is not really trivial
(at least it's not what client code is going to bother with).  So,
when user has a good old filename, he goes with
CAIRO_MIME_TYPE_XFILENAME, and if we see this in SVG backend, we do
%-encode it.  Other backends, if they handle
CAIRO_MIME_TYPE_XFILENAME, might need some encoding for this or might
not.

When user knows he has a valid URI he'll use CAIRO_MIME_TYPE_XURI, and
if we see this in SVG backend we won't break things for the user as
we're not going to encode it one more time.

>> 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 ;-).

OK, so let's wait for decision to keep the URI stuff in cairo.

>>
>> Is this looking any good? :)
>
> Yes. :-)

Thanks for your comments!

--
Alex