From 4909dd3b2c93d70b5d06782c239439fcf983f92e Mon Sep 17 00:00:00 2001 From: Axel Kittenberger Date: Tue, 3 Jan 2017 15:30:13 +0100 Subject: [PATCH] fixing racecondition in default.rsyncssh --- ChangeLog | 6 +- default-rsync.lua | 11 +- default-rsyncssh.lua | 277 ++++++++++++++++++++++++++++++------------- lsyncd.lua | 21 +++- tests/testlib.lua | 10 +- 5 files changed, 221 insertions(+), 104 deletions(-) diff --git a/ChangeLog b/ChangeLog index 38af51b..794aa0a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -21,10 +21,12 @@ change: _verbatim forced for 'exitcodes' entry. change: manpage is not rebuild by default. it is provided precompiled. - change: - faulty/deprecated config files that use settings = { ... }, with equal sign + change: faulty/deprecated config files that use settings = { ... }, with equal sign are no longer worked around. change: default.direct now calls copy with -p + fix: potential race conditions: + default.rsyncssh will now channel deletes also through rsync and treats moves + as blocking events. fix: ']' is not escaped for rsync rules, since rsync only applies doesn't applie pattern matching if no other pattern chars are found. diff --git a/default-rsync.lua b/default-rsync.lua index dd4d4eb..8af8767 100644 --- a/default-rsync.lua +++ b/default-rsync.lua @@ -170,6 +170,8 @@ rsync.action = function ( inlet ) + local config = inlet.getConfig( ) + -- gets all events ready for syncing local elist = inlet.getEvents( eventNotInitBlank ) @@ -198,13 +200,11 @@ rsync.action = function table.insert( filterI, path ) end + -- adds a path to the filter. -- - -- Adds a path to the filter. - -- - -- Rsync needs to have entries for all steps in the path, + -- rsync needs to have entries for all steps in the path, -- so the file for example d1/d2/d3/f1 needs following filters: -- 'd1/', 'd1/d2/', 'd1/d2/d3/' and 'd1/d2/d3/f1' - -- for _, path in ipairs( paths ) do if path and path ~= '' @@ -216,6 +216,7 @@ rsync.action = function while pp do addToFilter( pp ) + pp = string.match( pp, '^(.*/)[^/]+/?' ) end end @@ -231,8 +232,6 @@ rsync.action = function filterS ) - local config = inlet.getConfig( ) - local delete = nil if config.delete == true diff --git a/default-rsyncssh.lua b/default-rsyncssh.lua index 75314b0..d50cf36 100644 --- a/default-rsyncssh.lua +++ b/default-rsyncssh.lua @@ -69,6 +69,71 @@ rsyncssh.checkgauge = { } } + +-- +-- Returns true for non Init, Blanket and Move events. +-- +local eventNotInitBlankMove = + function +( + event +) + -- TODO use a table + if event.etype == 'Move' + or event.etype == 'Init' + or event.etype == 'Blanket' + then + return 'break' + else + return true + end +end + + +-- +-- Replaces what rsync would consider filter rules by literals. +-- +local replaceRsyncFilter = + function +( + path +) + if not path + then + return + end + + return( + path + :gsub( '%?', '\\?' ) + :gsub( '%*', '\\*' ) + :gsub( '%[', '\\[' ) + ) +end + + +-- +-- Mutates paths for rsync filter rules, +-- changes deletes to multi path patterns +-- +local pathMutator = + function +( + etype, + path1, + path2 +) + if string.byte( path1, -1 ) == 47 + and etype == 'Delete' + then + return replaceRsyncFilter( path1 ) .. '***', replaceRsyncFilter( path2 ) + else + return replaceRsyncFilter( path1 ), replaceRsyncFilter( path2 ) + end +end + + + -- -- Spawns rsync for a list of events -- @@ -76,10 +141,10 @@ rsyncssh.action = function ( inlet ) - local event, event2 = inlet.getEvent( ) - local config = inlet.getConfig( ) + local event, event2 = inlet.getEvent( ) + -- makes move local on target host -- if the move fails, it deletes the source if event.etype == 'Move' @@ -117,114 +182,148 @@ rsyncssh.action = function -- uses ssh to delete files on remote host -- instead of constructing rsync filters - if event.etype == 'Delete' - then - if config.delete ~= true - and config.delete ~= 'running' - then - inlet.discardEvent( event ) +-- if event.etype == 'Delete' +-- then +-- if config.delete ~= true +-- and config.delete ~= 'running' +-- then +-- inlet.discardEvent( event ) +-- +-- return +-- end +-- +-- -- gets all other deletes ready to be +-- -- executed +-- local elist = inlet.getEvents( +-- function( e ) +-- return e.etype == 'Delete' +-- end +-- ) +-- +-- -- returns the paths of the delete list +-- local paths = elist.getPaths( +-- function( etype, path1, path2 ) +-- if path2 +-- then +-- return config.targetdir..path1, config.targetdir..path2 +-- else +-- return config.targetdir..path1 +-- end +-- end +-- ) +-- +-- -- ensures none of the paths is '/' +-- for _, v in pairs( paths ) +-- do +-- if string.match( v, '^%s*/+%s*$' ) +-- then +-- log( 'Error', 'cowardly refusing to `rm -rf /` the target!' ) +-- +-- terminate( -1 ) -- ERRNO +-- end +-- end +-- +-- log( +-- 'Normal', +-- 'Deleting list\n', +-- table.concat( paths, '\n' ) +-- ) +-- +-- local params = { } +-- +-- spawn( +-- elist, +-- config.ssh.binary, +-- '<', table.concat(paths, config.xargs.delimiter), +-- params, +-- config.ssh._computed, +-- config.host, +-- config.xargs.binary, +-- config.xargs._extra +-- ) +-- +-- return +-- end + -- otherwise a rsync is spawned + local elist = inlet.getEvents( eventNotInitBlankMove ) + + -- gets the list of paths for the event list + -- deletes create multi match patterns + local paths = elist.getPaths( pathMutator ) + + -- stores all filters by integer index + local filterI = { } + + -- stores all filters with path index + local filterP = { } + + -- adds one path to the filter + local function addToFilter + ( + path + ) + if filterP[ path ] + then return end - -- gets all other deletes ready to be - -- executed - local elist = inlet.getEvents( - function( e ) - return e.etype == 'Delete' - end - ) + filterP[ path ] = true - -- returns the paths of the delete list - local paths = elist.getPaths( - function( etype, path1, path2 ) - if path2 - then - return config.targetdir..path1, config.targetdir..path2 - else - return config.targetdir..path1 - end - end - ) - - -- ensures none of the paths is '/' - for _, v in pairs( paths ) - do - if string.match( v, '^%s*/+%s*$' ) - then - log( 'Error', 'cowardly refusing to `rm -rf /` the target!' ) - - terminate( -1 ) -- ERRNO - end - end - - log( - 'Normal', - 'Deleting list\n', - table.concat( paths, '\n' ) - ) - - local params = { } - - spawn( - elist, - config.ssh.binary, - '<', table.concat(paths, config.xargs.delimiter), - params, - config.ssh._computed, - config.host, - config.xargs.binary, - config.xargs._extra - ) - - return + table.insert( filterI, path ) end + -- adds a path to the filter. -- - -- for everything else a rsync is spawned - -- - local elist = inlet.getEvents( - function( e ) - -- TODO use a table - return( - e.etype ~= 'Move' - and e.etype ~= 'Delete' - and e.etype ~= 'Init' - and e.etype ~= 'Blanket' - ) - end - ) - - local paths = elist.getPaths( ) - - -- - -- removes trailing slashes from dirs. - -- - for k, v in ipairs( paths ) + -- rsync needs to have entries for all steps in the path, + -- so the file for example d1/d2/d3/f1 needs following filters: + -- 'd1/', 'd1/d2/', 'd1/d2/d3/' and 'd1/d2/d3/f1' + for _, path in ipairs( paths ) do - if string.byte( v, -1 ) == 47 + if path and path ~= '' then - paths[k] = string.sub( v, 1, -2 ) + addToFilter( path ) + + local pp = string.match( path, '^(.*/)[^/]+/?' ) + + while pp + do + addToFilter( pp ) + + pp = string.match( pp, '^(.*/)[^/]+/?' ) + end end end - local sPaths = table.concat( paths, '\n' ) + local filterS = table.concat( filterI, '\n' ) - local zPaths = table.concat( paths, '\000' ) + local filter0 = table.concat( filterI, '\000' ) log( 'Normal', 'Rsyncing list\n', - sPaths + filterS ) + local delete = nil + + if config.delete == true + or config.delete == 'running' + then + delete = { '--delete', '--ignore-errors' } + end + spawn( elist, config.rsync.binary, - '<', zPaths, + '<', filter0, config.rsync._computed, + '-r', + delete, + '--force', '--from0', - '--files-from=-', + '--include-from=-', + '--exclude=*', config.source, config.host .. ':' .. config.targetdir ) @@ -352,6 +451,14 @@ rsyncssh.prepare = function ) end + if config.maxProcesses ~= 1 + then + error( + 'default.rsyncssh must have maxProcesses set to 1.', + level + ) + end + local cssh = config.ssh; cssh._computed = { } diff --git a/lsyncd.lua b/lsyncd.lua index 9910ba8..6a39362 100644 --- a/lsyncd.lua +++ b/lsyncd.lua @@ -1705,12 +1705,10 @@ local InletFactory = ( function -- -- Gets all events that are not blocked by active events. -- - -- @param if not nil a function to test each delay - -- getEvents = function ( - sync, - test + sync, -- the sync of the inlet + test -- if not nil use this function to test if to include an event ) local dlist = sync:getDelays( test ) @@ -2555,8 +2553,19 @@ local Sync = ( function for _, d in self.delays:qpairs( ) do - if d.status == 'active' - or ( test and not test( InletFactory.d2e( d ) ) ) + local tr = true + + if test + then + tr = test( InletFactory.d2e( d ) ) + end + + if tr == 'break' + then + break + end + + if d.status == 'active' or not tr then getBlocks( d ) elseif not blocks[ d ] diff --git a/tests/testlib.lua b/tests/testlib.lua index c2fc4a8..7952a7a 100644 --- a/tests/testlib.lua +++ b/tests/testlib.lua @@ -39,11 +39,11 @@ function mktempd local s = f:read( '*a' ) f:close( ) - + s = s:gsub( '[\n\r]+', ' ' ) - + s = s:match( '^%s*(.-)%s*$' ) - + return s end @@ -414,7 +414,7 @@ function churn local function mvfile ( ) local odir, fn, c = pickFile( ) - + if not odir then return @@ -464,7 +464,7 @@ function churn end local dice - + if init then dice =