GNOME Bugzilla – Bug 777894
Avoid mixing files with paths
Last modified: 2017-02-20 16:00:34 UTC
This was fixed in 8f04b292c0e551e2ba8dba2c77012699e40685c7, but I've just reverted it because it broke structre paths containing '..', like: /Documents/1/Structure/DocStructure.struct ../FixedDoc.fdoc#PG_441_LNK_2464 this is now resolved as: /Documents/1/Structure/../FixedDoc.fdoc#PG_441_LNK_2464 and it used to be resolved as: /Documents/1/FixedDoc.fdoc#PG_441_LNK_2464 We need this, because we compare paths directly, that's why we need to resolve them. The reason why we used GFile as because g_file_for_path canonicalize the path. The problem is that the method canonicalize_filename is private in glib and uses G_DIR_SEPARATOR_S and G_IS_DIR_SEPARATOR. A possible solution wold be to keep the existing code and then replace \ with / #ifdef G_OS_WIN32. Or could do the canonicalization ourselves, by copying the canonicalize_filename() method and replacing all G_DIR_SEPARATOR_S by "/" and G_IS_DIR_SEPARATOR by p == '/'.
Btw, you can reproduce this by opening the XPS spec in evince, for example. You sere the outline is collapsed and you and error if you try to navigate it.
Created attachment 344496 [details] [review] parse-utils: instead of using GFile use paths and canonalize them This takes the method from glib to canonalize the paths just that instead of using G_DIR_SEPARATOR it always uses / since we want / for all the platforms.
Carlos I went for the option of copying the method since IMHO it is more correct. Also when discussing with pbor the other option would endup being a big hack.
Review of attachment 344496 [details] [review]: ::: libgxps/gxps-parse-utils.c @@ +428,3 @@ + */ + i = 0; + for (p = start - 1; (p >= canon) && G_IS_DIR_SEPARATOR (*p); p--) Does this work in windows? Shouldn't we also check here if *p == '/' explicitly instead of using G_IS_DIR_SEPARATOR?
(In reply to Carlos Garcia Campos from comment #4) > Review of attachment 344496 [details] [review] [review]: > > ::: libgxps/gxps-parse-utils.c > @@ +428,3 @@ > + */ > + i = 0; > + for (p = start - 1; (p >= canon) && G_IS_DIR_SEPARATOR (*p); p--) > > Does this work in windows? Shouldn't we also check here if *p == '/' > explicitly instead of using G_IS_DIR_SEPARATOR? We can change it if you want but on windows it is defined like this: #define G_IS_DIR_SEPARATOR(c) ((c) == G_DIR_SEPARATOR || (c) == '/') so we should be safe
Created attachment 346270 [details] [review] parse-utils: instead of using GFile use paths and canonalize them This takes the method from glib to canonalize the paths just that instead of using G_DIR_SEPARATOR it always uses / since we want / for all the platforms.
(In reply to Ignacio Casal Quinteiro (nacho) from comment #5) > (In reply to Carlos Garcia Campos from comment #4) > > Review of attachment 344496 [details] [review] [review] [review]: > > > > ::: libgxps/gxps-parse-utils.c > > @@ +428,3 @@ > > + */ > > + i = 0; > > + for (p = start - 1; (p >= canon) && G_IS_DIR_SEPARATOR (*p); p--) > > > > Does this work in windows? Shouldn't we also check here if *p == '/' > > explicitly instead of using G_IS_DIR_SEPARATOR? > > We can change it if you want but on windows it is defined like this: > #define G_IS_DIR_SEPARATOR(c) ((c) == G_DIR_SEPARATOR || (c) == '/') > > so we should be safe I took your advice and changed it too since it is definitely more correct.
This problem has been fixed in the unstable development version. The fix will be available in the next major software release. You may need to upgrade your Linux distribution to obtain that newer version.