From 7adf456afb226d0ccd94eac80ec86ed1761888cf Mon Sep 17 00:00:00 2001 From: Thierry Gosselin Date: Sun, 6 Sep 2026 14:09:53 +1000 Subject: [PATCH 1/2] Exit unsuccessfully when FASTQ parsing detects malformed records Use error_exit instead of returning NULL when a FASTQ record has an invalid separator or unequal sequence and quality lengths. Returning NULL made these detected errors indistinguishable from normal EOF, allowing incomplete processing to finish with exit status 0. Preserve existing record diagnostics and include the input filename in the fatal error message. This is separate from filesystem read-error handling proposed in PR #533. --- src/fastqreader.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/fastqreader.cpp b/src/fastqreader.cpp index 00867fe7..c2076a76 100644 --- a/src/fastqreader.cpp +++ b/src/fastqreader.cpp @@ -347,8 +347,7 @@ Read* FastqReader::read(){ if (strand->empty() || (*strand)[0]!='+') { cerr << *name << endl; cerr << "Expected '+', got " << *strand << endl; - cerr << "Your FASTQ may be invalid, please check the tail of your FASTQ file" << endl; - return NULL; + error_exit("Invalid FASTQ separator in: " + mFilename); } if(quality->length() != sequence->length()) { @@ -357,8 +356,7 @@ Read* FastqReader::read(){ cerr << *sequence << endl; cerr << *strand << endl; cerr << *quality << endl; - cerr << "Your FASTQ may be invalid, please check the tail of your FASTQ file" << endl; - return NULL; + error_exit("FASTQ sequence/quality length mismatch in: " + mFilename); } if(readInPool) From a6a068e207080fc616203febd8a2be7da173e3de Mon Sep 17 00:00:00 2001 From: Thierry Gosselin Date: Sun, 6 Sep 2026 14:10:52 +1000 Subject: [PATCH 2/2] Add regression tests for FASTQ parser error exits Test invalid or missing separators and short, long, or missing quality strings in plain and gzipped FASTQ files. Place malformed records both at the beginning and after 40,000 valid reads. Verify that valid input, including files without a final newline, continues to work. All 24 cases pass with the fix. Unmodified Conda fastp 1.3.6 incorrectly returns success for all 20 malformed-input cases. --- scripts/test_fastq_parse_errors.py | 55 ++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 scripts/test_fastq_parse_errors.py diff --git a/scripts/test_fastq_parse_errors.py b/scripts/test_fastq_parse_errors.py new file mode 100644 index 00000000..f3bb36d5 --- /dev/null +++ b/scripts/test_fastq_parse_errors.py @@ -0,0 +1,55 @@ +#!/usr/bin/env python3 +"""Run: python3 scripts/test_fastq_parse_errors.py ./fastp.""" +import gzip +import pathlib +import subprocess +import sys +import tempfile + + +def main(): + exe = str(pathlib.Path(sys.argv[1]).resolve()) + record = b'@valid\nACGTACGTACGTACGTACGT\n+\nIIIIIIIIIIIIIIIIIIII\n' + # Place errors beyond initial sampling as well as at the first record. + cases = [ + ('valid', record*4, None), + ('no_final_newline', (record*4).rstrip(b'\n'), None), + ('invalid_separator', b'@bad\nACGT\nwrong\nIIII\n', 'Invalid FASTQ separator'), + ('missing_separator', b'@bad\nACGT\n', 'Invalid FASTQ separator'), + ('short_quality', b'@bad\nACGT\n+\nIII\n', 'FASTQ sequence/quality length mismatch'), + ('long_quality', b'@bad\nACGT\n+\nIIIII\n', 'FASTQ sequence/quality length mismatch'), + ('missing_quality', b'@bad\nACGT\n+\n', 'FASTQ sequence/quality length mismatch'), + ] + failures = 0 + with tempfile.TemporaryDirectory(prefix='fastp-parse-test-') as tmp: + root = pathlib.Path(tmp) + for zipped in (False, True): + for name, data, expected_error in cases: + for late in ((False, True) if expected_error else (False,)): + label = '%s-%s-%s' % (name, zipped, late) + content = record*40000 + data if late else data + source = root/(label + ('.fq.gz' if zipped else '.fq')) + output = root/(label+'.out.fq') + source.write_bytes(gzip.compress(content) if zipped else content) + cmd = [exe, '-i', str(source), '-o', str(output), '-w', '2', + '-A', '-Q', '-L', '-G', '--dont_eval_duplication', + '-j', '/dev/null', '-h', '/dev/null'] + try: + run = subprocess.run(cmd, capture_output=True, timeout=30) + if expected_error: + ok = (run.returncode > 0 and + expected_error.encode() in run.stderr and + str(source).encode() in run.stderr) + else: + ok = (run.returncode == 0 and output.exists() and + output.read_bytes() == content.rstrip(b'\n')+b'\n') + detail = 'exit=%d' % run.returncode + except subprocess.TimeoutExpired: + ok, detail = False, 'timeout' + print('%s %s %s' % ('PASS' if ok else 'FAIL', label, detail)) + failures += not ok + return bool(failures) + + +if __name__ == '__main__': + sys.exit(main())