Removed redundant/buggy basename and dirname functions. Added parse_redir_string to www controller.

Removed basename and dirname from mvc and fs, use posix functions instead.
parse_path_info was buggy and used 2 ways, rewrote and created parse_redir_string for backwards parsing.

6 files changed, 65 insertions, 51 deletions

diff --git a/app/acf-util/logon-controller.lua b/app/acf-util/logon-controller.lua
index f1fa93d..165bedb 100644
--- a/app/acf-util/logon-controller.lua
+++ b/app/acf-util/logon-controller.lua
@@ -34,8 +34,8 @@ logon = function(self)
 		if logon.value then
 			-- only copy the logonredirect if redirecting to that page
 			if logonredirect and cmdresult.value.redir.value then
-				local prefix, controller, action = self.parse_path_info("/"..cmdresult.value.redir.value)
-				if logonredirect.action == action and logonredirect.controller == controller then
+				local prefix, controller, action = self.parse_redir_string(cmdresult.value.redir.value)
+				if logonredirect.action == action and logonredirect.controller == controller and logonredirect.prefix == prefix then
 					self.sessiondata.logonredirect = logonredirect
 				end
 			end
diff --git a/app/acf_www-controller.lua b/app/acf_www-controller.lua
index ee7f344..2913f1e 100644
--- a/app/acf_www-controller.lua
+++ b/app/acf_www-controller.lua
@@ -66,7 +66,7 @@ local check_permission = function(self, prefix, controller, action)
 end
 
 local check_permission_string = function (self, str)
-	local prefix, controller, action = self.parse_path_info("/" .. (str or ""))
+	local prefix, controller, action = parse_redir_string(str)
 	if prefix == "/" then prefix = self.conf.prefix end
 	if controller == "" then controller = self.conf.controller end
 	
@@ -98,10 +98,10 @@ find_template = function ( appdir, prefix, controller, action, viewtype )
 		end
 	end
 	-- not found, so try one level higher
-	if prefix == "" then -- already at the top level - fail
+	if prefix == "." then -- already at the top level - fail
 		return nil
 	end
-	prefix = dirname (prefix) 
+	prefix = posix.dirname (prefix) 
 	return find_template ( appdir, prefix, controller, action, viewtype )
 end
 
@@ -145,7 +145,7 @@ local dispatch_component = function(self, str, clientdata, suppress_view)
 	self.clientdata = clientdata or {}
 	self.clientdata.sessionid = tempclientdata.sessionid
 
-	local prefix, controller, action = self.parse_path_info("/" .. str)
+	local prefix, controller, action = parse_redir_string(str)
 	if prefix == "/" then prefix = self.conf.prefix end
 	if controller == "" then controller = self.conf.controller end
 	local viewtable = self.dispatch(self, prefix, controller, action)
@@ -332,6 +332,7 @@ exception_handler = function (self, message )
 			parent_exception_handler(self, message)
 		end
 	else
+		logevent("Exception: "..message)
 		viewtable = {message = message}
 		self.conf.prefix = "/"
 		self.conf.controller = "exception"
@@ -373,7 +374,8 @@ dispatch = function (self, userprefix, userctlr, useraction)
 	end
 
 	-- This is for get / post data saved for after logon
-	if self.sessiondata.logonredirect and self.conf.controller == self.sessiondata.logonredirect.controller
+	if self.sessiondata.logonredirect and self.conf.prefix == self.sessiondata.logonredirect.prefix
+	and self.conf.controller == self.sessiondata.logonredirect.controller
 	and self.conf.action == self.sessiondata.logonredirect.action then
 		ENV.HTTP_REFERER = self.sessiondata.logonredirect.referrer or ENV.HTTP_REFERER
 		self.clientdata = self.sessiondata.logonredirect.clientdata
@@ -481,7 +483,7 @@ redirect = function (self, str, result)
 	if result then
 		self.sessiondata[self.conf.action.."result"] = result
 	end
-	local prefix, controller, action = self.parse_path_info("/" .. (str or ""))
+	local prefix, controller, action = parse_redir_string(str)
 	if prefix ~= "/" then self.conf.prefix = prefix end
 	if controller ~= "" then self.conf.controller = controller end
 	
@@ -506,7 +508,7 @@ redirect_to_referrer = function(self, result)
 			end
 		else
 			local prefix, controller, action = self.parse_path_info(ENV.HTTP_REFERER:gsub("%?.*", ""))
-			if controller ~= self.conf.controller or action ~= self.conf.action then
+			if prefix ~= self.conf.prefix or controller ~= self.conf.controller or action ~= self.conf.action then
 				self.sessiondata[self.conf.action.."result"] = result
 				error({type="redir_to_referrer"})
 			end
@@ -520,6 +522,25 @@ redirect_to_referrer = function(self, result)
 	return result
 end
 
+-- parse a "URI" like string into a prefix, controller and action
+-- this is the same as URI string, but opposite preference
+-- if only one is defined, it's assumed to be the action
+parse_redir_string = function( str )
+	str = str or "" 
+	str = string.gsub(str, "/+$", "")
+	local action = string.match(str, "[^/]+$") or ""
+	str = string.gsub(str, "/*[^/]*$", "")
+	local controller = string.match(str, "[^/]+$") or ""
+	str = string.gsub(str, "/*[^/]*$", "")
+	local prefix = string.match(str, "[^/]+$") or ""
+	if prefix == "" then
+		prefix = "/"
+	else
+		prefix = "/"..prefix.."/"
+	end
+	return prefix, controller, action
+end
+
 -- FIXME - need to think more about this..
 logevent = function ( message )
 	conf.logfile:write (string.format("%s: %s\n", os.date(), message or "")) 
diff --git a/lib/fs.lua b/lib/fs.lua
index 1dab796..2f45e21 100644
--- a/lib/fs.lua
+++ b/lib/fs.lua
@@ -10,22 +10,6 @@ module (..., package.seeall)
 require("posix")
 require("format")
 
-basename = function (string, suffix)
-	string = string or ""
-	local basename = string.gsub (string, "[^/]*/", "")
-	if suffix then 
-		basename = string.gsub ( basename, suffix, "" )
-	end
-	return basename 
-end
-
-dirname = function ( string)
-	string = string or ""
-	local basename = basename(string)
-	string = string.sub(string, 1, #string - #basename - 1)
-	return string	
-end 
-
 -- generic wrapper funcs
 function is_dir ( pathstr )
 	return posix.stat ( pathstr or "", "type" ) == "directory"
@@ -71,7 +55,7 @@ end
 -- Creates a blank file (and the directory if necessary)
 function create_file ( path )
 	path = path or ""
-	if dirname(path) and not posix.stat(dirname(path)) then create_directory(dirname(path)) end
+	if posix.dirname(path) and not posix.stat(posix.dirname(path)) then create_directory(posix.dirname(path)) end
 	local f = io.open(path, "w")
 	if f then f:close() end
 	return is_file(path)
@@ -80,11 +64,11 @@ end
 -- Copies a file to a directory or new filename (creating the directory if necessary)
 -- fails if new file is already a directory
 function copy_file(oldpath, newpath)
-	if not is_file(oldpath) or not newpath or newpath == "" or (basename(newpath) ~= "" and is_dir(newpath)) or (basename(newpath) == "" and is_dir(newpath .. basename(oldpath))) then
+	if not is_file(oldpath) or not newpath or newpath == "" or (posix.basename(newpath) ~= "." and is_dir(newpath)) or (posix.basename(newpath) == "." and is_dir(newpath .. posix.basename(oldpath))) then
 		return false
 	end
-	if dirname(newpath) and not posix.stat(dirname(newpath)) then create_directory(dirname(newpath)) end
-	if basename(newpath) == "" then newpath = newpath .. basename(oldpath) end
+	if posix.dirname(newpath) and not posix.stat(posix.dirname(newpath)) then create_directory(posix.dirname(newpath)) end
+	if posix.basename(newpath) == "." then newpath = newpath .. posix.basename(oldpath) end
 	local old = io.open(oldpath, "r")
 	local new = io.open(newpath, "w")
 	new:write(old:read("*a"))
@@ -96,11 +80,11 @@ end
 -- Moves a file to a directory or new filename (creating the directory if necessary)
 -- fails if new file is already a directory
 function move_file(oldpath, newpath)
-	if not is_file(oldpath) or not newpath or newpath == "" or (basename(newpath) ~= "" and is_dir(newpath)) or (basename(newpath) == "" and is_dir(newpath .. basename(oldpath))) then
+	if not is_file(oldpath) or not newpath or newpath == "" or (posix.basename(newpath) ~= "." and is_dir(newpath)) or (posix.basename(newpath) == "." and is_dir(newpath .. posix.basename(oldpath))) then
 		return false
 	end
-	if dirname(newpath) and not posix.stat(dirname(newpath)) then create_directory(dirname(newpath)) end
-	if basename(newpath) == "" then newpath = newpath .. basename(oldpath) end
+	if posix.dirname(newpath) and not posix.stat(posix.dirname(newpath)) then create_directory(posix.dirname(newpath)) end
+	if posix.basename(newpath) == "." then newpath = newpath .. posix.basename(oldpath) end
 	local status, errstr, errno = os.rename(oldpath, newpath)
         -- errno 18 means  Invalid cross-device link
 	if status or errno ~= 18 then
@@ -146,7 +130,7 @@ end
 -- write a string to a file, will replace file contents
 function write_file ( path, str )
 	path = path or ""
-	if dirname(path) and not posix.stat(dirname(path)) then create_directory(dirname(path)) end
+	if posix.dirname(path) and not posix.stat(posix.dirname(path)) then create_directory(posix.dirname(path)) end
 	local file = io.open(path, "w")
 	--append a newline char to EOF
 	str = string.gsub(str or "", "\n*$", "\n")
@@ -160,7 +144,7 @@ end
 -- fs.write_line_file ("filename", "Line1 \nLines2 \nLines3")
 function write_line_file ( path, str )
 	path = path or ""
-	if dirname(path) and not posix.stat(dirname(path)) then create_directory(dirname(path)) end
+	if posix.dirname(path) and not posix.stat(posix.dirname(path)) then create_directory(posix.dirname(path)) end
 	local file = io.open(path)
 	if ( file) then
 		local c = file:read("*a") or ""
@@ -179,7 +163,7 @@ function find_files_as_array ( what, where, follow, t )
 	if follow and fs.is_link(where) then
 		link = posix.readlink(where)
 		if not string.find(link, "^/") then
-			link = dirname(where).."/"..link
+			link = posix.dirname(where).."/"..link
 		end
 	end
 
@@ -195,7 +179,7 @@ function find_files_as_array ( what, where, follow, t )
 				table.insert (t, ( string.gsub ( where .. "/" .. d, "/+", "/" ) ) )
 			end
 		end
-	elseif (string.match (basename(where), "^" .. what .. "$" ))  then
+	elseif (string.match (posix.basename(where), "^" .. what .. "$" ))  then
 		table.insert (t, where )
 	end
 
diff --git a/lib/menubuilder.lua b/lib/menubuilder.lua
index 8c93795..51ed76d 100644
--- a/lib/menubuilder.lua
+++ b/lib/menubuilder.lua
@@ -5,6 +5,7 @@
   ]]--
 module(..., package.seeall)
 
+require("posix")
 require("format")
 
 -- returns a table of the "*.menu" tables 
@@ -63,8 +64,8 @@ get_menuitems = function (startdir)
 	local reversecats = {}
 	startdir = (string.gsub(startdir, "/$", ""))	--remove trailing /
 	for k,filename in pairs(get_candidates(startdir)) do
-		local controller = mvc.basename(filename, ".menu")
-		local prefix = (string.gsub(mvc.dirname(filename), startdir, "")).."/"
+		local controller = string.gsub(posix.basename(filename), ".menu$", "")
+		local prefix = (string.gsub(posix.dirname(filename), startdir, "")).."/"
 
 		-- open the menu file, and parse the contents
 		local handle = io.open(filename)
diff --git a/lib/roles.lua b/lib/roles.lua
index 9d89f0c..f38ce6a 100644
--- a/lib/roles.lua
+++ b/lib/roles.lua
@@ -170,7 +170,7 @@ local determine_perms = function(self,roles)
 		if reverseroles[entry.id] then
 			temp = format.string_to_table(entry.entry, ",")
 			for z,perm in pairs(temp) do
-				local prefix,control,action = mvc.parse_path_info(perm)
+				local prefix,control,action = self.parse_path_info(perm)
 				if control then
 					if nil == permissions[prefix] then
 						permissions[prefix] = {}
diff --git a/www/cgi-bin/mvc.lua b/www/cgi-bin/mvc.lua
index d7824d1..4aeb8fa 100755
--- a/www/cgi-bin/mvc.lua
+++ b/www/cgi-bin/mvc.lua
@@ -6,6 +6,8 @@
   ]]--
 module(..., package.seeall)
 
+require("posix")
+
 mvc = {}
 
 -- the constructor
@@ -155,7 +157,7 @@ dispatch = function (self, userprefix, userctlr, useraction)
 	-- Because of the inheritance, normally the 
 	-- controller.worker.action will flow up, so that all children have
 	-- actions of all parents.  We use rawget to make sure that only
-	-- controller defined aictions are used on dispatch
+	-- controller defined actions are used on dispatch
 	-- If the action is missing, raise an error
 	if ( type(rawget(controller.worker, action)) ~= "function") then
 		self.conf.type = "dispatch"
@@ -215,8 +217,8 @@ soft_require = function (self, name )
 		-- and if it doesnt exist silently fail.
 		-- This version allows things from /usr/local/lua/5.1 to
 		-- be loaded
-		package.path = self.conf.appdir .. dirname(name) .. "/?.lua;" .. package.path
-		local t = require(basename(name))
+		package.path = self.conf.appdir .. posix.dirname(name) .. "/?.lua;" .. package.path
+		local t = require(posix.basename(name))
 		package.path = PATH
 		return t
 	end
@@ -254,7 +256,7 @@ read_config = function( self, appname )
 	for i, filename in ipairs (confs) do
                 local file = io.open (filename)
                 if (file) then
-			self.conf.confdir = dirname(filename) .. "/"
+			self.conf.confdir = posix.dirname(filename) .. "/"
                         for line in file:lines() do
                                 key, value = string.match(line, "([^[=]*)=[ \t]*(.*)")
                                 if key then -- ugly way of finding blank spots between key and =
@@ -289,15 +291,17 @@ end
 -- return them (or blank strings)
 parse_path_info = function( str )
 	str = str or "" 
-	-- If it ends in a /, then add another to force
-	-- a blank action (the user gave a controller without action)
-	if  string.match (str, "[^/]/$" ) then 
-		str = str .. "/"
+	local words = {}
+	str = string.gsub(str, "/+$", "")
+	for x=1,3 do
+		words[#words+1] = string.match(str, "[^/]+$")
+		str = string.gsub(str, "/+[^/]*$", "")
 	end
-	local action = basename(str)
-	local temp = dirname(str)
-	local controller = basename(temp)
-	local prefix = dirname(temp) .. "/"
+	prefix = "/"..(words[#words] or "").."/"
+	if prefix == "//" then prefix = "/" end
+	controller = words[#words-1] or ""
+	action = words[#words-2] or ""
+
 	return prefix, controller, action
 end
 
@@ -354,3 +358,7 @@ cfe = function ( optiontable )
 	return me
 end
 _G.cfe = cfe
+
+logevent = function ( ... )
+	os.execute ( "logger \"" .. (... or "") .. "\"" )
+end