Menu

#90 Filelocker does not behave correctly when config file option is not passed

v2
closed
None
9
2017-02-17
2016-10-28
bill halpin
No

In Filelocker2.py's main routine, there is logic to choose the appropriate configuration file location when one is not specified on the command line, however the logic is completely ignored because 'options.configfile' is passed to all the subroutines rather than 'configfile' where the proper location is stored.

Discussion

  • David Hutchins

    David Hutchins - 2016-11-14

    You are missing an important line, spicifically "cherrypy.config.update(configfile)", this pushes the variable configfile into the config object that gets passed to the other methods. see below:

    def start(configfile=None, daemonize=False, pidfile=None):
    cherrypy.file_uploads = dict()
    cherrypy.file_downloads = dict()
    if configfile is None:
    configfile = os.path.join(os.getcwd(),"etc","filelocker.conf")
    cherrypy.config.update(configfile)
    logLevel = 40

     
  • David Hutchins

    David Hutchins - 2016-11-14
    • status: open --> closed
    • assigned_to: David Hutchins
     
  • David Hutchins

    David Hutchins - 2016-11-14

    Closed, there is no bug here. the default config file loction is passed along through the config object in cherrypy. [directory of Filelocker.py]/etc/filelocker.conf

     

    Last edit: David Hutchins 2016-11-14
  • bill halpin

    bill halpin - 2017-02-16

    I didnt miss anything. You are referring to a completely different block of code. If your intention is not to default /etc/filelocker.conf when a path is not passed and a configfile does not exist in the local directory, why does this logic exist in main:

    if options.configfile is not None:
    configParser.read(options.configfile)
    else:
    configfile = os.path.join(os.getcwd(),"etc","filelocker.conf")
    if not os.path.exists(configfile):
    configfile = os.path.join("/","etc","filelocker.conf")

    As you can see, if you do not pass a path, and a local config file doesnt exist, the code should default to /etc/filelocker.conf, however since you manipulate a local variable and not options.configfile, the correct value is not passed:

        elif options.action == "start":
            start(options.configfile, options.daemonize, options.pidfile)
    

    At the very least, the logic in both of those functions should be identical. If /etc/filelocker.conf is not a valid 'default' location it should not be tested for in main and should throw the error, if it is valid, it should pass that value correctly.

     
    • David Hutchins

      David Hutchins - 2017-02-16

      Ohh I see now. I will re-open this, I am not the original dev here, so
      there are parts I'm still figuring out.

       

      Last edit: David Hutchins 2017-02-17
  • David Hutchins

    David Hutchins - 2017-02-16
    • status: closed --> open
     
  • David Hutchins

    David Hutchins - 2017-02-17

    Fixed in: https://sourceforge.net/p/filelocker2/git/ci/95225ae64e87c5d65473babb26e3eafbbceb075d/

    Tested with:
    * no file in any default location - PASS throws exception
    * <pwd>/etc/filelocker.conf - PASS reads file
    * /etc/filelocker.conf.example - PASS reads file
    * specified existing file using -c - PASS reads file
    * specified non-existing file using -c - FAIL attempts to read and continues</pwd>

    Updated: https://sourceforge.net/p/filelocker2/git/ci/6ee8fe8dac547761edabd82d4c97642d9d694af1/

    Tested with:
    * no file in any default location - PASS throws exception
    * <pwd>/etc/filelocker.conf - PASS reads file
    * /etc/filelocker.conf.example - PASS reads file
    * specified existing file using -c - PASS reads file
    * specified non-existing file using -c - PASS throws exception</pwd>

     

    Last edit: David Hutchins 2017-02-17
  • David Hutchins

    David Hutchins - 2017-02-17
    • status: open --> closed
     

Log in to post a comment.