*** Bug 708645 has been marked as a duplicate of this bug. ***

Using realpath with NULL isn't portable either, afaik.

realpath with NULL is defined in POSIX.1-2008.

(In reply to comment #2)
> Using realpath with NULL isn't portable either, afaik.

It may be but it is at least more portable than canonicalize_file_name which is only available on Linux imho.

Hi Christian.

Any final input on this? Would you prefer a patch with check for canonicalize_file_name()?
Thank you.

In the past I've had at least one bug report where realpath with NULL didn't work.

An alternative solution would be to use gnulib which has a module for this function.

(In reply to comment #6)
> An alternative solution would be to use gnulib which has a module for this
> function.

Bleh... I never used gnulib before. Is it supposed to be bundled within a project or does it needs to be installed separately then looked for?

Created attachment 258810 [details] [review]
Simply avoid canonicalizing file names.

This patch reimplements part of _rsvg_handle_allow_load, simply avoiding the calls to canonicalize_file_name by using g_file_has_prefix to do the comparison. This takes care of every use of canonicalize_file_name in the code.

My code should be equivalent, but I haven't actually tested it except for the fact that it compiles.

I think this is probably a better approach than trying to use a canonicalize_file_name substitute.

That seems to work for me :-)

Christian any comment on that approach? It would be nice to have this fixed once and for all. Thank you.

*** Bug 719742 has been marked as a duplicate of this bug. ***

Thanks!

The patch that was committed is attachment 257324 [details] [review].

As discussed above, the function realpath with null passed in as the second argument isn't portable either. iirc, it will segfault on solaris. Additionally, the function realpath doesn't exist at all on windows.

I'm therefore reopening this, as the underlying bug (lack of portability) still has not been fixed.

(In reply to comment #13)
> The patch that was committed is attachment 257324 [details] [review].
> 
attachment 263366 [details] [review]
It will validate the behavior of realpath in configure.ac, and fallback to canonicalize_file_name if the former fails and canonicalize_file_name is available on the platform, otherwise throw a compiling error.

I think that canonicalize_file_name is only really available on platforms where realpath(path, NULL) also works. So, that patch wouldn't really gain anything in terms of portability.

(In reply to comment #15)
> I think that canonicalize_file_name is only really available on platforms where
> realpath(path, NULL) also works.
canonicalize_file_name is not available on Mac OS X, where realpath(path, NULL) is there.

And realpath(path, NULL) is specified in POSIX.1-2008: http://man7.org/linux/man-pages/man3/realpath.3.html

I know. What I'm saying is that there's never a reason to use canonicalize_file_name.

canonicalize_file_name only seems to exist on platforms where realpath(path, NULL) works correctly.

Well, on a platform that doesn't support canonicalize_file_name, old version of librsvg will throw a compiling error.

On a platform that has realpath but not support realpath(path, NULL), current librsvg will not throw a compiling error, leave the user to debug SEGV or other errors.

So checking the behavior of realpath(path, NULL) in configure shall make sense, though it is not possible in cross-compiling case. In cross-compiling, we can use canonicalize_file_name or throw a compiling error(if no canonicalize_file_name).

(In reply to comment #13)
> As discussed above, the function realpath with null passed in as the second
> argument isn't portable either. iirc, it will segfault on solaris.

Have you checked that this is still true on current illumos?

No, to be honest I don't know much about the solaris/illumos case, I just remember someone mentioning that it segfaulted.

I do know that if you want to support realpath on windows, you need to add code like this (which will work with the null case):

#ifdef G_OS_WIN32
#define realpath(a,b) _fullpath(b,a,_MAX_PATH)
#endif

Although, I still haven't heard anyone review my patch (attachment 258810 [details] [review]). It seems simpler to me than using realpath. Is there any reason you refuse to use it?

I ran into this issue when compiling for Windows/MinGW.

Would be great if a solution can be found. Attachment 258810 [details] seems to work on Windows. If there are no security considerations that might be a workable solution?

Review of attachment 258810 [details] [review]:

I have also run into this issue and the proposed patch makes sense to me.

However I suggest calling g_file_new_from_uri near the beginning of the function and then calling g_file_get_uri_scheme instead of g_uri_parse_scheme, this way we avoid parsing the uri twice

Actually after discussing this on irc, realpath does i/o to resolve symlinks, while working with gfile does not. So if the attacker symlinks /a/b -> /c then the gfile prefix check for /a/c has-prefix /a will succeed while the realpah won't.

We either need to implement a realpath equivalent using gio (if there isn't one already) or we need to go with one of the other solutions like that of comment #20 (if it works)

Created attachment 283702 [details] [review]
Use _fullpath() on Windows for realpath()

Hi,

As the upcoming GTK+-3.14 release series will need to use librsvg in some way at run time, either directly via a GDK-Pixbuf rsvg loader or indirectly via the recently-added gtk-encode-symbolic-svg, I think it would be fantastic if we can have librsvg-2.40.2+ working for Windows.  I am attaching a patch for this in accordance to Mike's suggestions in comment #20, for references.

With blessings, thanks a bunch!

I tested the patch in comment 24, and I can confirm that it fixes compilation for windows.

I've pushed Chun-wei's patch to git master. Thanks!

Created attachment 300037 [details] [review]
W32: Use _wfullpath+g_convert to get UTF-8 support

(In reply to LRN from comment #27)
> Created attachment 300037 [details] [review] [review]
> W32: Use _wfullpath+g_convert to get UTF-8 support

Thanks!  I committed the following variation, as all of Glib uses g_utf8_to_utf16() for this kind of G_OS_WIN32 special case.

+static char *
+rsvg_realpath_utf8 (const char *filename, const char *unused)
+{
+    wchar_t *wfilename;
+    wchar_t *wfull;
+    char *full;
+
+    wfilename = g_utf8_to_utf16 (filename, -1, NULL, NULL, NULL);
+    if (!wfilename)
+        return NULL;
+
+    wfull = _wfullpath (NULL, wfilename, 0);
+    g_free (wfilename);
+    if (!wfull)
+        return NULL;
+
+    full = g_utf16_to_utf8 (wfull, -1, NULL, NULL, NULL);
+    free (wfull);
+
+    if (!full)
+        return NULL;
+
+    return full;
+}
+
+#define realpath(a,b) rsvg_realpath_utf8 (a, b)

This is in commit 28694c791067f0321ae21d44734b1ef88b45d742