Skip to content

Refuse a symlink where the parsed output goes, and actually write it - #504

Merged
chrismattmann merged 1 commit into
verify-the-server-jar-before-running-itfrom
refuse-a-symlink-where-the-output-goes
Nov 9, 2026
Merged

chrismattmann merged 1 commit into
verify-the-server-jar-before-running-itfrom
refuse-a-symlink-where-the-output-goes

Conversation

@chrismattmann

Copy link
Copy Markdown
Owner

Stacked on #502 — based on that branch, so this diff shows only the new change. Merge #502 first.

Two things in parseAndSave, both in the same three lines.

It did not write anything without an outDir

The write sat inside the else:

if outDir is None:
    metaPath = path + metaExtension          # <- and that was all
else:
    metaPath = os.path.join(outDir, ...)
    log.info('Writing %s' % metaPath)
    with open(metaPath, 'w', encoding='utf-8') as f:
        f.write(...)

So parseAndSave(option, paths) computed a path, wrote nothing, and returned the name of a file that did not exist — in a function called parseAndSave, whose docstring says it "write[s] extracted metadata and/or text in JSON format to matching filename".

This is a behaviour change. Callers who passed no outDir previously got no files and will now get <input>_meta.json next to each input, which is what the name and the docstring promise. Say the word if you'd rather I split this out or leave the old behaviour.

A symlink at the output name was written through

The destination name is derived from the input name, so it is predictable, and a link planted at it got written through as whoever runs tika. Same shape as the log and cache files in #502, and it applies to both branches — the no-outDir case writes next to the input, which is just as predictable.

Unlike the cache, this path is deliberately not treated as ours:

  • Ownership is not checked. The directory is the caller's to choose and may legitimately be group-writable or shared; refusing that would break ordinary use. The symlink refusal is the part that matters here.
  • The mode stays 0o666, so the umask decides, exactly as the plain open() did. These are outputs somebody asked for, not cache files — forcing 0600 would quietly change who can read them.

_openPrivate grows checkOwner, encoding and perm for that distinction, plus a text 'w' mode so the utf-8 encoding the old call specified is preserved.

Tests

tests/test_output_paths.py, 10 cases; suite 68 → 78, all passing. The symlink case was confirmed to fail against the plain open() rather than assumed:

FAILED tests/test_output_paths.py::test_a_symlink_at_the_output_name_is_not_written_through

Also pinned: that the opener's two callers differ the way they're meant to — a cache file lands 0600 and ownership-checked, an output file does not; that the umask still decides output permissions; that an existing output is overwritten rather than appended; and that utf-8 survives.

Two things in parseAndSave, both in the same three lines.

The write sat inside the `else` for the outDir case. So parseAndSave with no
outDir computed a path, wrote nothing, and returned the name of a file that
did not exist -- in a function called parseAndSave whose docstring says it
writes the extracted metadata. It now writes in both branches. This is a
behaviour change: callers who passed no outDir previously got no files, and
will now get <input>_meta.json next to each input, which is what the name and
the docstring promise.

The destination is opened without following a symlink at the final component.
The output name is derived from the input name, so it is predictable, and a
link planted at it was written through as whoever runs tika. That is the same
shape as the log and cache files, and it applies to both branches -- the
no-outDir case writes next to the input, which is just as predictable.

Unlike the cache, this is not treated as ours:

  - ownership is not checked. The directory is the caller's to choose and may
    legitimately be group-writable or shared, and refusing that would break
    ordinary use. The symlink refusal is the part that matters here.
  - the mode stays 0o666 so the umask decides, exactly as the plain open() it
    replaces did. These are outputs somebody asked for, not cache files, and
    forcing 0600 would quietly change who can read them.

_openPrivate takes checkOwner, encoding and perm for that distinction, and a
text 'w' mode so the utf-8 encoding the old call specified is preserved.

tests/test_output_paths.py, 10 cases. Suite 68 -> 78. The symlink case was
confirmed to fail against the plain open() rather than assumed:

  FAILED test_a_symlink_at_the_output_name_is_not_written_through

Also pinned: that both callers of the opener differ the way they are supposed
to -- a cache file lands 0600 and ownership-checked, an output file does not.
@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage is 72.818% — refuse-a-symlink-where-the-output-goes into verify-the-server-jar-before-running-it. No base build found for verify-the-server-jar-before-running-it.

@chrismattmann chrismattmann self-assigned this Nov 9, 2026
@chrismattmann chrismattmann added this to the tika-next milestone Nov 9, 2026
@chrismattmann
chrismattmann merged commit 85ce51b into verify-the-server-jar-before-running-it Nov 9, 2026
12 checks passed
@chrismattmann chrismattmann mentioned this pull request Nov 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants