Re: [NSE][PATCH] datafiles.lua

[Sven Klemm <sven () c3d2 de>]

Hi jah,

thanks for testing my patch.

> On 09/09/2008 19:51, Sven Klemm wrote:
> > The attached patch contains a few modifications to datafiles.lua:
> > Make get_array() and get_assoc_array() normal functions. They were
> > local functions in parse_lines() but there was no reason for them to
> > be local functions and moving them out makes parse_lines() easier to
> > read.
> I didn't consider these functions useful enough to be exposed through
> the library, but there's no other reason why they couldn't be.  Doing so
> does indeed make the code a little easier to read.

Yes I thought so. We could still make them module local functions if 
we don't want to get them exported.

> > Remove check for "(" in supplied pattern strings. I think the check is
> > superfluous as a pattern does not need to have a "(" to produce a
> > match as the complete match will be returned if there is no match
> > group specified and strings containing "(" do not necessarily produce
> > a match as a string might contain a literal "(" .
> The check for a "(" is necessary to determine whether a string is a
> literal or a pattern for matching.  The parse_lines() function needs to
> be able to reject certain combinations of key, value so that it doesn't
> waste time returning a table of junk - which would happen, for example,
> if the table passed was something like { ["hello"] = "nmap" }.  This
> means that there is a requirement to include captures in the pattern
> string passed to parse_file() or parse_lines() and is the only solution
> I could come-up with to distinguish between a string literal and a pattern.
>
> With your patch, making the call datafiles.parse_file( "nmap-services" )
> fails with "Error in datafiles.parse_lines: Invalid value for index
> udp." because this distinction isn't being made.
>
> The distinction between a string literal and a pattern allows us to pass
> a table such as { ["tcp"] = "pattern", ["udp"] = "pattern" } which
> returns tables such as the one returned by the original parse_services().
>
> I think it's unlikely that a string literal used for these kinds of
> purposes would contain parentheses and with this limitation, the
> functions should work nicely.

Ah I didn't think of that case. I've modified the patch to handle this 
situation by checking for value being a table. If value is a table 
then anything supplied as index will be used literally.

I don't see { ["hello"] = "nmap" } as problem if a script author wants 
a file parsed that way he should get it. I am currently using 
{["^[a-f0-9]+"]=""} in my SSH weak key script. I could easily modify 
it to make it work with the current version but I thought the check 
was too constraining. I am using an empty string as value because I am 
only interested in a hash table with strings as key for easy checking 
whether a key is in the file.

Cheers,
Sven

--
Sven Klemm
http://cthulhu.c3d2.de/~sven/

Index: nselib/datafiles.lua
===================================================================
--- nselib/datafiles.lua        (revision 10060)
+++ nselib/datafiles.lua        (working copy)
@@ -125,14 +125,8 @@
     data_struct = t
   end
 
-  -- get path to file
-  local filepath = nmap.fetchfile( filename )
-  if not filepath then
-    return false, ( "Error in nmap.fetchfile: Could not find file %s." ):format( filename )
-  end
-
   -- get a table of lines
-  local status, lines = read_from_file( filepath )
+  local status, lines = read_from_file( filename )
   if not status then
     return false, ( "Error in datafiles.parse_file: %s could not be read: %s." ):format( filepath, lines )
   end
@@ -168,91 +162,51 @@
 
   local ret = {}
 
-  -- return an array-like table of values captured from each line
-  function get_array( v_pattern )
-    local ret = {}
-    for _, line in ipairs( lines ) do
-      -- only process strings
-      if type( line ) == "string" then
-        local captured
-        if type( v_pattern ) == "function" then
-          captured = v_pattern( line )
-        else
-          captured = line:match( v_pattern )
-        end
-        ret[#ret+1] = captured
-      end
-    end
-    return ret
-  end
-
-  -- return an associative array table of index-value pairs captured from each line
-  function get_assoc_array( i_pattern, v_pattern )
-    local ret = {}
-    for _, line in ipairs(lines) do
-      -- only process strings
-      if type( line ) == "string" then
-        if type(i_pattern) == "function" then
-          index = i_pattern(line)
-        else
-          index = line:match(i_pattern)
-        end
-        if index and type(v_pattern) == "function" then
-          ret[index] = v_pattern(line)
-        elseif index then
-          ret[index] = line:match(v_pattern)
-        end
-      end
-    end
-    return ret
-  end
-
-
   -- traverse data_struct and enforce sensible index-value pairs.  Call functions to process the members of lines.
   for index, value in pairs( data_struct ) do
     if type(index) == nil then return false, "Error in datafiles.parse_lines: Invalid index." end
-    if type(index) == "number" or ( type(index) == "string" and not index:match("%(") ) then
-      if type(value) == "number" or ( type(value) == "string" and not value:match("%(") ) then
+    if type(index) == "number" or type(value) == "table" then
+      if type(value) == "number" then
         return false, "Error in datafiles.parse_lines: No patterns for data capture."
       elseif type(value) == "string" or type(value) == "function" then
-        ret = get_array( value )
+        ret = get_array( lines, value )
       elseif type(value) == "table" then
         _, ret[index] = parse_lines( lines, value )
       else
         -- TEMP
-        print(type(index), "unexpected value", type(value))
+        stdnse.print_debug("Unexpected value %s", type(value))
       end
     elseif type(index) == "string" or type(index) == "function"  then
       if type( value ) == "string" or type( value ) == "function" then
-        ret = get_assoc_array( index, value )
+        ret = get_assoc_array( lines, index, value )
       else
         return false, ( "Error in datafiles.parse_lines: Invalid value for index %s." ):format( index )
       end
     else
       -- TEMP
-      print("unexpexted index", type(index), type(value))
+      stdnse.print_debug("unexpexted index %s %s", type(index), type(value))
     end
   end
 
-
   return true, ret
-
 end
 
 
 ---
 -- Reads a file, line by line, into a table.
--- @param file  String representing a filepath.
+-- @param file  String with the name of the file to read.
 -- @return      Boolean True on success, False on error
 -- @return      Table (array-style) of lines read from the file or error message in case of an error.
 
 function read_from_file( file )
 
-  if type( file ) ~= "string" or file == "" then
-    return false, "Error in datafiles.read_from_file: Expected file as a string."
+  -- get path to file
+  local filepath = nmap.fetchfile( file )
+  if not filepath then
+    return false, ( "Error in nmap.fetchfile: Could not find file %s." ):format( filename )
   end
 
-  local f, err, _ = io.open( file, "r" )
+  local f, err, _ = io.open( filepath, "r" )
   if not f then
     return false, ( "Error in datafiles.read_from_file: Cannot open %s for reading: %s" ):format( file, err )
   end
@@ -270,3 +224,43 @@
 
 end
 
+--- return an array-like table of values captured from each line
+--@param lines table of strings containing the lines to process
+--@param v_pattern pattern to use on the lines to produce the value for the array
+get_array = function( lines, v_pattern )
+  local ret = {}
+  for _, line in ipairs( lines ) do
+    assert( type( line ) == "string" )
+    local captured
+    if type( v_pattern ) == "function" then
+      captured = v_pattern( line )
+    else
+      captured = line:match( v_pattern )
+    end
+    table.insert( ret, captured )
+  end
+  return ret
+end
+
+--- return an associative array table of index-value pairs captured from each line
+--@param lines table of strings containing the lines to process
+--@param i_pattern pattern to use on the lines to produce the key for the associative array
+--@param v_pattern pattern to use on the lines to produce the value for the associative array
+get_assoc_array = function( lines, i_pattern, v_pattern )
+  local ret = {}
+  for _, line in ipairs(lines) do
+    assert( type( line ) == "string" )
+    if type(i_pattern) == "function" then
+      index = i_pattern(line)
+    else
+      index = line:match(i_pattern)
+    end
+    if index and type(v_pattern) == "function" then
+      ret[index] = v_pattern(line)
+    elseif index then
+      ret[index] = line:match(v_pattern)
+    end
+  end
+  return ret
+end
+


_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://SecLists.Org