Skip to content

Fall back to the Python strings when the system tool refuses a file - #5908

Open
arpitjain099 wants to merge 1 commit into
ossf:mainfrom
arpitjain099:fix/strings-subprocess-failure
Open

Fall back to the Python strings when the system tool refuses a file#5908
arpitjain099 wants to merge 1 commit into
ossf:mainfrom
arpitjain099:fix/strings-subprocess-failure

Conversation

@arpitjain099

Copy link
Copy Markdown

Closes #5907.

parse_strings called subprocess.check_output(["strings", ...]) with no failure handling, so a non-zero exit from the system tool raised CalledProcessError out of the scan. Strings, the pure-Python implementation sitting directly above it, reads bytes and does not care what container format the file claims to be, so the fallback was already there and simply was not reached.

I reproduced this on macOS rather than taking it on trust. A real class file, JREProperties.class from a LibreOffice install, gives the reported error verbatim:

error: strings: fat file: JREProperties.class truncated or malformed
(offset plus size of cputype (6556160) cpusubtype (775) extends past the end of the file)

and parse_strings on it raised CalledProcessError before this change and now returns 727 characters, including java/lang/Object and java/lang/System. /bin/ls still goes down the fast path and returns the same 1424 characters either way, so the system tool is still doing the work where it can.

One thing beyond the report. check_output inherits stderr, so a jar full of class files printed one error: ... fat file: line per entry straight to the console. stderr is captured into the exception now and logged at debug, which keeps the diagnostic without the noise.

Only a non-zero exit is absorbed. OSError still propagates, since a tool that cannot be run at all is a different problem from one that has looked at the file and declined it, and quietly swallowing that would hide a broken environment. There is a test for that.

Three tests in test/test_strings.py, all against a generated class file so nothing binary is added to the repo: the fallback path, a control that the system tool is still used when it exits 0, and the OSError case. The first fails on the current code with the CalledProcessError.

The two pre-existing TestStrings cases fail here before and after this branch, because they compare against the system strings and BSD output differs from binutils. ruff check and ruff format --check are clean.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant