FLTK logo

STR #3441

FLTK matrix user chat room
(using Element browser app)   FLTK gitter user chat room   GitHub FLTK Project   FLTK News RSS Feed  
  FLTK Apps      FLTK Library      Forums      Links     Login 
 Home  |  Articles & FAQs  |  Bugs & Features  |  Documentation  |  Download  |  Screenshots  ]
 

Return to Bugs & Features | Roadmap 1.3 | SVN ⇄ GIT ]

STR #3441

Application:FLTK Library
Status:1 - Closed w/Resolution
Priority:3 - Moderate, e.g. unable to compile the software
Scope:3 - Applies to all machines and operating systems
Subsystem:Core Library
Summary:fl_filename_relative returns a wrong path
Version:1.3.4
Created By:nakane
Assigned To:matt
Fix Version:1.4.0
Fix Commit:c41b3a1a647155ab9c727dab168baf08e7609372
Update Notification:

Receive EMails Don't Receive EMails

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 ]

 
 

Comments are owned by the poster. All other content is copyright 1998-2024 by Bill Spitzak and others. This project is hosted by The FLTK Team. Please report site problems to 'erco@seriss.com'.