Re: [nmap-svn] r32366 - nmap-exp/d33tah/luaexec-lookup/ncat

[Jacek Wielemborek <wielemborekj1 () gmail com>]

2013/9/20  <commit-mailer () nmap org>:
> Author: d33tah
> Date: Fri Sep 20 15:16:22 2013
> New Revision: 32366
>
> Log:
> Move script lookup from ncat_lua.c to ncat_main.c to avoid passing
> argv0.
>
> Modified:
>    nmap-exp/d33tah/luaexec-lookup/ncat/ncat_core.c
>    nmap-exp/d33tah/luaexec-lookup/ncat/ncat_core.h
>    nmap-exp/d33tah/luaexec-lookup/ncat/ncat_lua.c
>    nmap-exp/d33tah/luaexec-lookup/ncat/ncat_main.c
>    nmap-exp/d33tah/luaexec-lookup/ncat/ncat_win.c
>
> Modified: nmap-exp/d33tah/luaexec-lookup/ncat/ncat_core.c
> ==============================================================================
> --- nmap-exp/d33tah/luaexec-lookup/ncat/ncat_core.c     (original)
> +++ nmap-exp/d33tah/luaexec-lookup/ncat/ncat_core.c     Fri Sep 20 15:16:22 2013
> @@ -211,8 +211,6 @@
>      o.sslverify = 0;
>      o.ssltrustfile = NULL;
>  #endif
> -
> -    o.argv0 = NULL;
>  }
>
>  /* Internal helper for resolve and resolve_numeric. addl_flags is ored into
> @@ -579,14 +577,3 @@
>              break;
>      }
>  }
> -
> -/* This is basically copied from nmap.c - perhaps it should be moved to nbase? */
> -char* executable_dir()
> -{
> -    char *dir, *path;
> -    path = executable_path(o.argv0);
> -    if (path == NULL)
> -        return path;
> -    dir = path_get_dirname(path);
> -    return dir;
> -}
>
> Modified: nmap-exp/d33tah/luaexec-lookup/ncat/ncat_core.h
> ==============================================================================
> --- nmap-exp/d33tah/luaexec-lookup/ncat/ncat_core.h     (original)
> +++ nmap-exp/d33tah/luaexec-lookup/ncat/ncat_core.h     Fri Sep 20 15:16:22 2013
> @@ -203,8 +203,6 @@
>      char *sslkey;
>      int sslverify;
>      char *ssltrustfile;
> -
> -    char *argv0;
>  };
>
>  extern struct options o;
> @@ -269,5 +267,3 @@
>
>  extern int setenv_portable(const char *name, const char *value);
>  extern void setup_environment(struct fdinfo *fdinfo);
> -
> -char* executable_dir();
>
> Modified: nmap-exp/d33tah/luaexec-lookup/ncat/ncat_lua.c
> ==============================================================================
> --- nmap-exp/d33tah/luaexec-lookup/ncat/ncat_lua.c      (original)
> +++ nmap-exp/d33tah/luaexec-lookup/ncat/ncat_lua.c      Fri Sep 20 15:16:22 2013
> @@ -153,36 +153,12 @@
>
>  void lua_setup(void)
>  {
> -    int loadfile_result;
>      ncat_assert(o.cmdexec != NULL);
>
>      L = luaL_newstate();
>      luaL_openlibs(L);
>
> -    loadfile_result = luaL_loadfile(L, o.cmdexec);
> -
> -    if (loadfile_result == LUA_ERRFILE) {
> -        char *exe_path, *script_path;
> -        int script_path_len;
> -        exe_path = executable_dir();
> -        if (exe_path == NULL)
> -            bye("Could not find the script file.");
> -        script_path_len = strlen(exe_path) + strlen(o.cmdexec) + 32;
> -        script_path = (char *)safe_malloc(script_path_len);
> -#ifdef WIN32
> -        Snprintf(script_path, script_path_len, "%s\\ncat_scripts\\%s", exe_path, o.cmdexec);
> -#else
> -        Snprintf(script_path, script_path_len, "%s/../share/ncat/scripts/%s", exe_path, o.cmdexec);
> -#endif
> -        if (o.debug)
> -            logdebug("Failed to load \"%s\" in the working directory, trying \"%s\"...\n",
> -                o.cmdexec, script_path);
> -        loadfile_result = luaL_loadfile(L, script_path);
> -        free(exe_path);
> -        free(script_path);
> -    }
> -
> -    if (loadfile_result != 0)
> +    if (luaL_loadfile(L, o.cmdexec) != 0)
>          report("Error loading the Lua script");
>
>      /* install the traceback function */
>
> Modified: nmap-exp/d33tah/luaexec-lookup/ncat/ncat_main.c
> ==============================================================================
> --- nmap-exp/d33tah/luaexec-lookup/ncat/ncat_main.c     (original)
> +++ nmap-exp/d33tah/luaexec-lookup/ncat/ncat_main.c     Fri Sep 20 15:16:22 2013
> @@ -148,7 +148,62 @@
>
>  #ifdef HAVE_LUA
>  #include "ncat_lua.h"
> +
> +static char* executable_dir(char *argv0)
> +{
> +    char *dir, *path;
> +    path = executable_path(argv0);
> +    if (path == NULL)
> +        return path;
> +    dir = path_get_dirname(path);
> +    return dir;
> +}
> +
> +static int try_open(char *filename)
> +{
> +    FILE *opened = fopen(filename, "r");
> +    if (opened == NULL) {
> +        return 0;
> +    } else {
> +        fclose(opened);
> +        return 1;
> +    }
> +}
> +
> +/* Try to find path for the script, looking in both the current directory and
> +   - if the lookup filed - in a directory relative to the Ncat's executable.
> +   Returns a dynamically allocated variable. */
> +static char* find_script_path(char *filename, char *argv0)
> +{
> +    char *exe_path, *script_path;
> +    int script_path_len;
> +
> +    if (try_open(filename))
> +        return filename;
> +
> +    exe_path = executable_dir(argv0);
> +    if (exe_path == NULL)
> +        bye("Could not find the script file.");
> +
> +    script_path_len = strlen(exe_path) + strlen(filename) + 32;
> +    script_path = (char *)safe_malloc(script_path_len);
> +#ifdef WIN32
> +    Snprintf(script_path, script_path_len, "%s\\ncat_scripts\\%s", exe_path, filename);
> +#else
> +    Snprintf(script_path, script_path_len, "%s/../share/ncat/scripts/%s", exe_path, filename);
>  #endif
> +    free(exe_path);
> +
> +    if (o.debug)
> +        logdebug("Failed to load \"%s\" in the working directory, trying \"%s\"...\n",
> +            filename, script_path);
> +
> +    if (!try_open(script_path))
> +        bye("Could not find the script file in any of the search directories.");
> +
> +    return script_path;
> +}
> +#endif /* HAVE_LUA */
>
>  static int ncat_connect_mode(void);
>  static int ncat_listen_mode(void);
> @@ -324,8 +379,6 @@
>      /* Set default options. */
>      options_init();
>
> -    o.argv0 = argv[0];
> -
>  #ifdef WIN32
>      windows_init();
>  #endif
> @@ -525,7 +578,7 @@
>              else if (strcmp(long_options[option_index].name, "lua-exec") == 0) {
>                  if (o.cmdexec != NULL)
>                      bye("Only one of --exec, --sh-exec, and --lua-exec is allowed.");
> -                o.cmdexec = optarg;
> +                o.cmdexec = find_script_path(optarg, argv[0]);
>                  o.execmode = EXEC_LUA;
>              } else if (strcmp(long_options[option_index].name, "lua-exec-internal") == 0) {
>                  /* This command-line switch is undocumented on purpose. Do NOT use it
> @@ -545,7 +598,7 @@
>                      perror("Cannot set mode");
>  #endif
>                  ncat_assert(argc == 3);
> -                o.cmdexec = argv[2];
> +                o.cmdexec = find_script_path(argv[2], argv[0]);
>                  lua_setup();
>                  lua_run();
>              }
>
> Modified: nmap-exp/d33tah/luaexec-lookup/ncat/ncat_win.c
> ==============================================================================
> --- nmap-exp/d33tah/luaexec-lookup/ncat/ncat_win.c      (original)
> +++ nmap-exp/d33tah/luaexec-lookup/ncat/ncat_win.c      Fri Sep 20 15:16:22 2013
> @@ -154,10 +154,12 @@
>      int n, rc;
>      size_t size, offset;
>
> -    bundlename = executable_dir();
> -    if (bundlename == NULL)
> +    /* Get the executable's filename. */
> +    n = GetModuleFileName(GetModuleHandle(0), buf, sizeof(buf));
> +    if (n == 0 || n == sizeof(buf))
>          return -1;
>
> +    bundlename = path_get_dirname(buf);
>      bundlename = (char *) safe_realloc(bundlename, 1024);
>      offset = strlen(bundlename);
>      size = offset + 1;
>
> _______________________________________________
> Sent through the svn mailing list
> http://nmap.org/mailman/listinfo/svn

You said:

Now, I want you to split up lua_setup, into one function that just
initializes the lua_State, and another function where you give it a
script name. It's not good to have so much application logic (trying
multiple platform-specfic paths) in a function like lua_setup. Also, I
think you would be able to do this within main, which would mean you
don't have to push argv0 into the global scope. What you want is a
helper function in ncat_main.c, that first calls lua_setup, then calls
some other function with a name that you give it. Then you call this
helper function from main(). The idea is to keep these application
details close to the application and farther from the Lua library
implementation.

What do you think about the approach I just took?
_______________________________________________
Sent through the dev mailing list
http://nmap.org/mailman/listinfo/dev
Archived at http://seclists.org/nmap-dev/