[ Return to Bugs & Features | Roadmap 1.3 ]

STR #3441

Trouble Report Files:

	Name/Time/Date	Filename/Size
	#1  chris 05:06 Oct 18, 2018	STR-3441.diff 1k
	#2  chris 05:07 Oct 18, 2018	fl_filename_relative_testsuite.cxx 3k
	#3  greg.ercolano 01:46 Oct 25, 2018	fl_filename_relative--v1.cxx 13k

Trouble Report Comments:

	Name/Time/Date	Text
	#1  nakane 04:13 Dec 12, 2017	I found a bug in fl_filename_relative. #include <stdio.h> #include <FL/filename.H> int main(int argc, char **argv) {         char from[] = "/test/path/folder/file";         char base[] = "/test/path_suffix/folder";         char to[256] = "";         fl_filename_relative(to, 256, from, base);         printf("%s\n", to);         return 0; } This returns "../../folder/file" instead of "../../path/folder/file". I don't think this is OS specific, but my environment is:  Linux 3.10.0  gcc 4.8.5
	#2  nakane 04:27 Dec 12, 2017	#2384 says this was fixed but the problem still persists. (the patch in #2384 is included in 1.3.4)
	#3  greg.ercolano 19:27 Oct 17, 2018	Confirming OP's results. There's at least two versions of this function:     fl_filename_relative(to,tolen,from,base) // 4 params     fl_filename_relative(to,tolen,from)      // 3 params (base is cwd) Test case (actual unix screen history): _______________ # mkdir -p /test/path/folder ; echo hello > /test/path/folder/file # mkdir -p /test/path_suffix/folder && cd /test/path_suffix/folder # pwd /test/path_suffix/folder # more ../../folder/file                      <-- fltk's response ../../folder/file: No such file or directory  <-- FAIL # more ../../path/folder/file                 <-- expected response hello                                         <-- works ________________ Investigating..
	#4  greg.ercolano 22:43 Oct 17, 2018	Developer notes:     As the OP points out, this code has been serviced before and still seems buggy.     To get it right for sure this time, a possible alternative to fixing     FLTK's code: /replace/ it with time-tested code from the functionally     equivalent gnu core utility realpath(1), e.g:         # realpath -m --relative-to=/test/path_suffix/folder /test/path/folder/file         ../../path/folder/file     Source for realpath(1) can be found here:     https://github.com/coreutils/coreutils/tree/master/src     The internal function, relpath(), appears to be functionally equivalent.     This same code is also used in the ln(1) command.
	#5  greg.ercolano 04:47 Oct 18, 2018	Argh; dragons be there in using GPL code in our LGPL library. Looks like we have to re-invent the wheel after all.
	#6  chris 05:06 Oct 18, 2018	FWIW I had started on a patch that time, but during working on this STR I came upon another issue with multiple slashes in the path, which I tried to fix too. After that I really never managed to thorougly test it for regressions or side effects. Maybe it can serve as a starting point for someone, so I upload it here, together with a very basic test program...
	#7  greg.ercolano 06:03 Oct 18, 2018	Thanks Chris, yeah there's a couple of weird cases I can think of:   * a path that contains "..", e.g. "/var/log/../tmp/foo" == "/var/tmp/foo"   * On unix  "/var/tmp/foo" == "/var//tmp/foo" == "//var//tmp/foo" I haven't checked the windows code yet, but anticipate more such complexities with stuff like:    * "//svr/share/foo" == "//svr/share//foo" but not "/svr/share/foo"    * "//svr/share/foo" == "\\svr\share\foo"    * "c:\foo" == "C:\foo" == "c:/foo" == "C:/foo" == "c:/FOO" == etc ..and case variations may or may not work on macs, depending on if the file system mounts honor case or not (default is "not") I see we have separate code for win/unix
	#8  greg.ercolano 22:01 Oct 18, 2018	This is gonna take a rewrite I think. Canonicalizing fictional paths would mean fully parsing the filenames, and certainly a bit of work. realpath(3) provides canonicalizing, but not with fictional paths. realpath(3), a POSIX function, can canonicalize paths (removes extraneous /../, /./, and redundant sequences of "/"s) But the path has to exist; realpath(3) won't solve 'fictional' paths, whereas GNU realpath(1) can with its -m argument. (Not on BSD!) And realpath(3) touches the filesystem, which could be very slow when working with remote network paths (e.g. nfs, smbfs, AFP..)
	#9  greg.ercolano 01:46 Oct 25, 2018	Attaching fl_filename_relative--v1.cxx This is a /work in progress/ replacement for fl_filename_relative(). This code uses similar techniques that unix itself uses to resolve paths. This is a bit of work we have to do because unix itself doesn't provide API calls that let us do this type of thing easily. Chris, I also merged your regression tests with mine. TODO:     > This is unix only code; windows paths will be a separate can o'worms     > This code requires 'base' be a path to a directory (and not a file)     > Duplicate //'s and /./'s are handled by realpath(3), but it *only* works       if path actually exists. (Doesn't work for fictitious paths)
	#10  AlbrechtS 07:06 Jan 01, 2019	Reduced priority to 3 (moderate) since fl_filename_relative() is not an essential function of the library and this issue should not block the release of 1.3.5. @Greg: We should also consider moving this STR to 1.4.0 since it appears to me that the necessary code changes are likely too big to be applied to the 1.3.x branch which is end of life and should only get serious bug fixes. Do you agree? (If yes, feel free to move this STR to 1.4-current).
	#11  matt 04:54 Feb 05, 2019	Can I apply the patch? Has it been tested? Or do you want me to test it and patch the source?
	#12  greg.ercolano 09:49 Feb 05, 2019	@Albrecht comment #10, move to 1.4: yes. @Matt: I'm not sure how to answer. I think STR-3441 is not enough, and my V1 patch is possibly enough, though it needs some extra attention to the TODO items, more testing, and probably needs some #ifdef'ing for the different platform pathing (e.g. windows support for both \ and /, support for drive maps "C:\path", and UNC style paths e.g. \\unc\path and/or //unc/path)
	#13  AlbrechtS 09:12 Sep 27, 2023	This STR has been copied to GitHub Issue #781, please see https://github.com/fltk/fltk/issues/781 Please post follow-up's to this GitHub issue and close this STR when the bug is fixed.
	#14  matt 14:30 Oct 11, 2023	Fixed in Git repository.

[ Return to Bugs & Features ]