Skip to content

Use zcat instead of zless #2509

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
muffato wants to merge 1 commit into
base: develop
Choose a base branch
from
+2 −2
Open

Use zcat instead of zless #2509

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions misc/roh-viz
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ sub process_data
my ($opts) = @_;

print STDERR "Parsing $$opts{outdir}/roh.txt\n";
open(my $in,"zless $$opts{roh_file} |") or error("zless $$opts{roh_file}: $!");
open(my $in,"zcat $$opts{roh_file} |") or error("zcat $$opts{roh_file}: $!");
Copy link

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using zcat here will fail for the common case where --RoH-file is plain text (the help example uses roh.txt, not a .gz). zless can view uncompressed files, but zcat typically errors with “not in gzip format” and produces no parsed data. Consider opening the file directly when it’s not gzipped (e.g., based on extension or magic bytes), and only using a decompressor for .gz inputs (or use IO::Uncompress::Gunzip for gzip with a plain-file fallback).

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using one-arg open with a piped string command (open(my $in, "zcat $$opts{roh_file} |")) invokes a shell and lets it interpret $$opts{roh_file}, enabling OS command injection if that value is attacker-controlled. Here roh_file comes directly from CLI arguments, so a crafted filename containing shell metacharacters could execute arbitrary commands with this script’s privileges. Replace this with a shell-free invocation (e.g., multi-arg open or a decompression module that takes the filename as a separate argument) and/or strictly validate the allowed pathname characters.

Suggested change
open(my $in,"zcat $$opts{roh_file} |") or error("zcat $$opts{roh_file}: $!");
open(my $in, '-|', 'zcat', $$opts{roh_file}) or error("zcat $$opts{roh_file}: $!");

Copilot uses AI. Check for mistakes.
while (my $line=<$in>)
{
chomp($line);
Expand All @@ -175,7 +175,7 @@ sub process_data
push @{$$opts{roh}{$chr}{$smpl}},"$beg,$end,$nsnp,$qual";
}
}
close($in) or error("close failed: zless $$opts{in_file}");
close($in) or error("close failed: zcat $$opts{in_file}");
Copy link

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The close error message references $$opts{in_file}, but this function opens $$opts{roh_file} and there is no in_file option in this script. This will produce a misleading/empty error message if the pipe fails; update it to refer to the RoH input and ideally include the underlying failure info (e.g., $! and/or $?).

Suggested change
close($in) or error("close failed: zcat $$opts{in_file}");
close($in) or error("close failed: zcat $$opts{roh_file} (status=$?, error=$!)");

Copilot uses AI. Check for mistakes.

my $bin_size = $$opts{bin_size};
my %chr_len = ();
Expand Down
Loading