[Fontconfig] FcConfigNormalizeFontDir considered harmful

[Keith Packard]

On Mon, 2006-08-28 at 00:34 -0400, Patrick Lam wrote:

> I think that path normalization needs to run at least on strings that
> are input to fc-cache.  For instance, `fc-cache .' is going to create a
> cache file which claims to be for the directory .; not quite what we want.

Ok, so paths not starting with '/' should be cleaned up somehow,
probably by using something similar to the path normalization code, but
it needn't keep any state around (just rescan for each normalized path).

That's just for directory names that come from the user though, and
there aren't very many places where we have those aside from fc-cache.

> However, the original motivation for FcConfigNormalizeFontDir was the
> multiple path problem.  Not loading caches multiple times, but at least
> having incorrect names for directories and then putting them in the
> global cache.  I'm not sure that we need to run FcConfigNormalizeFontDir
> on startup to avoid that.

Yes, if you want to find the same cache file through multiple path
names, you would have to normalize on app startup. I'm not sure we care
that much though; a typical system doesn't generally produce multiple
font paths to the same font files.

> Since the global cache goes away, then all of the NormalizeFontDir uses
> in global cache code go away anyway.  That leaves calls in
> FcDirCacheUnlink, FcCacheReadDirs, FcDirCacheWrite, and FcDirScanConfig
> (aside from the call in fc-cache/fc-cache.c).

Sure, but any normalize call from anywhere causes the whole file system
to get scanned. Icky. Either we normalize all paths, or we normalize
none and expect people to just pass us consistent path names.

> There was some trickiness in calls to FcDirCacheUnlink due to multi-arch
> cache files.  Separating cache files on a per-arch basis will make that
> problem go away; after we separate them, then we can preemptively blow
> away cache files and nothing bad will happen.

heh. already done.

> The call in FcCacheReadDirs seems a bit strange.  One might
> want to normalize before calling FcConfigAcceptFilename to make sure
> that the masks do the right thing (they didn't in 2.3.2), because we can
> require that masks must be on canonical filenames (as stated in the
> <dir> entries).  But that's not what happens right now.  And otherwise,
> the arguments to FcCacheReadDirs should be, by definition, normalized,
> since they come from the <dir> entries in the config file.  So the call
> there just seems to prevent infinite loops, as per bug 2667 (and 4754).

I think we can have a recursion depth counter in the cache reading and
writing code and bail if it gets too high; that's a whole lot cheaper
than scanning for duplicates.

> Now FcDirScanConfig might create global caches with incorrect names if
> the paths are not normalized.  I haven't analyzed that case yet, but it
> seems like it could be potentially bad.

I suggest that any paths that don't start with '/' get cleaned up, and
otherwise we just leave them alone.

Here's a suggested algorithm for doing the normalization:

For relative paths:
   getcwd + pathname

Clean up path (removing ./ and ../ elements)

I don't think we need to get fancier than this; a simple recursion depth
check should catch any looping directory links.

-- 
keith.packard at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : http://lists.freedesktop.org/archives/fontconfig/attachments/20060827/8a3d71df/attachment.pgp