BrooksLabUCSC / flair

Full-Length Alternative Isoform analysis of RNA
Other
208 stars 71 forks source link

Escaping asterisks in file names #238

Closed chrisamiller closed 1 year ago

chrisamiller commented 1 year ago

There's an issue with escaping asterisks in ssCorrect.py. When run on human build38, it's possible to have a situation where an input contig name results in files like HLA-C*01:03_known_juncs.bed, but that also matches HLA-C*17:01:01:03_known_juncs.bed if the star is expanded as a glob (which the shell will do). This causes the call out to ssPrep.py to throw an error about unrecognized arguments (it's getting two where one is expected). This generally fails silently, but then eventually, causes ssCorrect.py to throw an error about a missing file (because ssPrep.py wasn't run correctly on this contig)

FileNotFoundError: [Errno 2] No such file or directory: '/tmp/nanodir/HLA-C*01:03_inconsistent.bed'

This can be resolved by modifying this line https://github.com/BrooksLabUCSC/flair/blob/7d0974c8061a7968097bb81246609d1d4d65cba6/src/flair/ssCorrect.py#L273

to be quoted properly:

    cmd = "%s %s -i '%s' -j '%s' -o '%s' --workingDir '%s' -f '%s' -w %s " % (sys.executable, helperScript, reads,juncs,prefix, tDir, f, wiggle)
diekhans commented 1 year ago

This approach could still fail with a path that had a quote in it. The prescribed way to protect quote a string to protect against shell interpretation is to use shlex.quote

a better approach would be for FLAIR to not send it through the shell using subprocess shell=True, which adds no value and risk (see Suprocess documentations).

chrisamiller commented 1 year ago

I'm not a pythonista, so feedback gratefully accepted. The PR I made with this fix can be closed if there's a better way!

diekhans commented 1 year ago

happy to advise;

import shlex ...

cmd = "%s %s -i %s -j %s -o %s --workingDir %s -f %s -w %s " % [shlex.quote(a) for a in (sys.executable, helperScript, reads,juncs,prefix, tDir, f, wiggle)]

would quote every argument using a list comprehension.

Chris Miller @.***> writes:

I'm not a pythonista, so feedback gratefully accepted. I'll close this, keep the issue open, and let you all figure out the details of implementation!

-- Reply to this email directly or view it on GitHub: https://github.com/BrooksLabUCSC/flair/issues/238#issuecomment-1366386348 You are receiving this because you commented.

Message ID: @.***> I'm not a pythonista, so feedback gratefully accepted. I'll close this, keep the issue open, and let you all figure out the details of implementation!

-- Reply to this email directly, [1]view it on GitHub, or [2]unsubscribe. You are receiving this because you commented. Message ID: @.***>

References

  1. https://github.com/BrooksLabUCSC/flair/issues/238#issuecomment-1366386348
  2. https://github.com/notifications/unsubscribe-auth/AAA23ZJR27WMXTVBOX6BWXTWPPICBANCNFSM6AAAAAATK57RD4
Jeltje commented 1 year ago

Thanks! The whole setup of calling the shell within flair is nails-on-chalkboard to me and I hope to change it to function calls sometime.