Directory fileOps are not processed when relPath ends in /

[reidsunderland]

Related issues:

• directory deletion with trailing slash causes problems with realpathAdjust sarrac#208
  • Fix #208 sarrac#209
• rmdir processing broken #1377
• directory removal race condition #1395

sr3 rejects messages with relPaths that end in /. When relPath ends in /, msg['new_file'] is an empty string ('') and that causes the message to be rejected by this code:

sarracenia/sarracenia/flow/__init__.py

Lines 1947 to 1953 in 52d5033

	if not 'new_dir' in msg or not msg['new_dir']:
	self.reject(msg, 422, f"new_dir message field missing, do not know which directory to put file in. skipping." )
	continue
	if not 'new_file' in msg or not msg['new_file']:
	self.reject(msg, 422, f"new_file message field missing, do not know name of file to write. skipping." )
	continue

This causes problems with directory (creation and removal) fileOps when the relPath ends in / as generated by sr3c/libshim when the input command also ends in slash (e.g. rm -r some_dir/ or mkdir some_dir/).

This was also not caught previously because the test case I wrote in sr3c creates and deletes the directory with a trailing slash -- so both the creation and removal messages are rejected by sr3:
https://github.com/MetPX/sarrac/blob/707d21d6a525ef8f3e19bffd0198e0a342e44aee/shim_copy_post.sh#L120-L125

---

The code that is currently written essentially does not allow relPaths ending in /. If we want to keep it that way, then we need to change sr3c to remove the slash when the message gets posted. Or sr3 (Python) could be modified to accept these messages.

[reidsunderland]

Workaround plugin:

"""
Workaround for https://github.com/MetPX/sarracenia/issues/1503

Directory fileOps that have relPath ending in / are rejected by sr3 because msg['new_file'] gets set to ''

This removes the trailing / before new_file is set.
"""
import logging
from sarracenia.flowcb import FlowCB

logger = logging.getLogger(__name__)

class Fix_dir_trailing_slash(FlowCB):
    def __init__(self, options):
        super().__init__(options, logger)

    def after_gather(self, worklist):
        for msg in worklist.incoming:
            if 'fileOp' in msg and 'directory' in msg['fileOp'] and msg['relPath'][-1] == '/':
                msg['relPath'] = msg['relPath'].rstrip('/')

[petersilva]

fwiw, I think a relPath that ends in ´/' is broken. So the C should be fixed to not produce it. On the other hand, Postel´s law (be liberal in what you accept) means it should be fixed in python as well.

[petersilva]

so... prepared two alternative patches. Only one should be used, not both:

1. This one 205e1f0 checks for a directory operation in the exact place in the code where the error is reported, and then adjusts new_dir and new_file for only that case.

2. On the other hand, this one: 3d988d4 goes way higher in the code so that when relPath, new_dir, and new_file processing is initially done, get rid of relPath's trailing slash. This is probably the better thing to do, but it has the potential for weird side effects.

They both pass the flow tests, but I don't have an easy test for the problem. I'll play with mirroring in hpc examples for a bit. I had an example where it was perfect, except all these empty directories left at the end. I'll see if one of these can fix that.

[reidsunderland]

Thanks!!

I want to double check if there are any cases where sr3 or v2 posts relPaths with a trailing slash. I doubt it, but I'll make sure.

I pushed a branch to the sarrac repo that changes the test to remove the false positive: MetPX/sarrac@e57cde0

make test_shim_copy_strip should be able to reproduce the problem/confirm a fix. I was playing with it yesterday but more urgent things used up all my time today :(

[reidsunderland]

It took me a long time to find time to test this, but 3d988d4 (higher in the code) works.

205e1f0 didn't work. It does too much adjustment. I pushed another commit (37231e4) to the issue1503_pas_justpatchit branch and that fixes it, but I haven't run flow tests.

I couldn't get sr3 watch to post any messages with relPaths ending in /. I haven't tested sr3_post.

I agree with fixing this problem in Python and sr3c, but I'm undecided on which Python fix is best.

I've been testing using make test_shim_copy_strip using the sr3c branch sr3_1503_tests