← All posts

Sanitising is not identifying: deriving a filename from untrusted data without collisions

Sanitising is not identifying: deriving a filename from untrusted data without collisions

Here is a job that sounds like an afternoon. A remote API hands you a list of items, each with a title and an id. You fetch each one and save it to disk. The filename should be readable, so it gets the title in it, and it has to be unique, so it gets the id too. Something like <title>.<id>.ext.

The title comes from strangers, so you sanitise it: strip the path separators, strip the control characters, make sure it can’t come out as . or ... Standard stuff, and there are a hundred blog posts about it.

Then you pass the id through the same function, because it also came from strangers, and you have just built a program that silently overwrites one item with another.

Two jobs that look identical and are opposites

Sanitising is lossy on purpose. That is the whole point: you have a large set of possible inputs and a small set of acceptable outputs, and you map the former onto the latter. / becomes _. So does \. So does anything else you don’t want in a name. Collapsing distinct inputs onto one output is not a side effect of sanitising, it is the definition of it.

Identifying is injective by requirement. Two distinct items must produce two distinct names, or the second one lands on the first one’s file. An injective function is one that never collapses two inputs into one output.

So the id is the one part of that filename that must not be sanitised, and it is also the part that looks most like it should be, because it arrived from the same untrusted place by the same route. That is what makes this an easy mistake to make and a hard one to see: the fix for the title is the bug for the id.

Every collision below is one I actually shipped, in a small tool of mine that saves files fetched from a remote provider and names them from the metadata that provider supplies. The threat model is milder than running code a model wrote, but the discipline is the same one: assume the bytes are hostile and decide what each field is allowed to do to your filesystem.

Five ways one filename ate another

a/b and a\b. Both separators map to the placeholder, so both ids sanitise to a_b. Two items, one name. This is the plain form of the problem and the easiest to fix badly, by adding more characters to the allowed set until the specific pair you noticed stops colliding.

The separator you chose is a character the id can contain. The name shape is <title>.<id>.ext, and a dot means “here is where the title ends”. Now consider title="a.b", id="c" against title="a", id="b.c". Both derive a.b.c.ext. Nothing was even mangled here; the name is simply ambiguous, and no amount of sanitising fixes an ambiguous grammar.

Two spellings of the same title. Unicode lets you write café as four codepoints with a precomposed é, or five with an e and a combining accent. They are different Python strings and they encode to 5 and 6 UTF-8 bytes:

>>> len(unicodedata.normalize("NFC", "café").encode("utf-8"))
5
>>> len(unicodedata.normalize("NFD", "café").encode("utf-8"))
6

Two paths for one item. The visible symptom was not a collision but its mirror image: the “do I already have this?” check derived the other spelling’s path, found nothing, and re-downloaded a file that was sitting on disk. So titles get NFC-normalised, which is the correct lossy move for a human-readable label, and it is exactly the move that must never be applied to the id, since normalising an identifier merges two items the caller handed you as distinct.

Slicing bytes. The limit here is a byte count and not a character count. os.pathconf('.', 'PC_NAME_MAX') returns 255 on this filesystem, and what the kernel rejected in practice was a 261-byte name, so the title has to be trimmed to fit in bytes. Both obvious ways are wrong:

>>> ("é" * 200).encode("utf-8")[:255].decode("utf-8")
UnicodeDecodeError: 'utf-8' codec can't decode byte ...

A byte slice can land in the middle of a multi-byte character. A character count fails in the other direction, because 200 characters can be 400 bytes. What works is to slice on bytes and then back off one byte at a time until it decodes, which is four lines and no cleverness.

Stripping once. strip(".") on ". . ." leaves " . ", and stripping whitespace off that leaves ".", which is a path component with a meaning you did not intend. The strip has to run to a fixpoint:

while True:
    stripped = text.strip().strip(".")
    if stripped == text:
        break
    text = stripped
return text or _PLACEHOLDER

The fix is reversibility

The id needs a transform that is injective, and the easiest way to guarantee injectivity is to pick something reversible, because a function you can invert cannot have collapsed anything. Percent-encoding every byte outside a small unreserved alphabet does it:

_ID_UNRESERVED = frozenset(string.ascii_letters + string.digits + "_-")

def encode_id(raw: str) -> str:
    if not raw:
        return _EMPTY_ID
    return "".join(
        chr(byte) if chr(byte) in _ID_UNRESERVED else f"%{byte:02X}"
        for byte in raw.encode("utf-8")
    )

That is RFC 3986 percent-encoding, “a character triplet, consisting of the percent character ‘%’ followed by the two hexadecimal digits representing that octet’s numeric value”, over a deliberately narrower unreserved set than the RFC’s own. The RFC’s is ALPHA / DIGIT / "-" / "." / "_" / "~"; mine drops the . and the ~, and both omissions are load-bearing below.

Nine lines, and it settles four of the five problems above at once:

  • a/b becomes a%2Fb and a\b becomes a%5Cb. Distinct in, distinct out.
  • . is deliberately outside the unreserved set, so it encodes to %2E and an encoded id can never contain a dot. That makes the name parse unambiguously from the right: the segment between the final two dots is the id, whatever the title got up to.
  • No normalisation happens, so the two spellings of one id stay two ids.
  • The ordinary case is untouched. Real ids from this provider are eleven characters of [A-Za-z0-9_-], so they encode to themselves and the readable filenames stay readable.

Note what the injectivity argument rests on. It is not “I tested the pairs I thought of”. It is a property of the grammar: two names being equal forces their id segments to be equal, and equal encodings force equal ids because the encoding is reversible. That argument covers inputs I never thought of, which is the only kind of coverage worth having against untrusted data.

Then the length limit ate the argument

Percent-encoding costs bytes. Three per byte of ASCII punctuation, six per two-byte accented character. An id of 100 dots encodes to 300 bytes, and the whole filename has 255 to work with, so the write fails with OSError [Errno 36] File name too long. My first version left the id uncapped on purpose, reasoning that a certain crash beats a silent collision, which I still think is the right ranking of those two outcomes.

It is not the choice, though, because truncation is not the only way to shorten something. Keep a readable prefix and append a digest of the whole raw id:

def bounded_encode_id(raw: str) -> str:
    encoded = encode_id(raw)
    if len(encoded) <= ID_MAX_BYTES:
        return encoded
    budget = ID_MAX_BYTES - len(_ID_SHORTENED_MARKER) - _ID_DIGEST_HEX
    prefix = _prefix_on_token_boundary(encoded, budget)
    digest = hashlib.blake2b(raw.encode("utf-8"), digest_size=8).hexdigest()
    return f"{prefix}{_ID_SHORTENED_MARKER}{digest}"

Two details in there matter more than the shortening.

The prefix is backed off to a %XX token boundary, because cutting %C3 into %C produces a string that is no longer valid percent-encoding, and the injectivity argument above depended on the encoding being decodable.

The marker is ~, and ~ is outside the output alphabet by construction: encode_id("~") is %7E. So a literal ~ in an id segment can only be the marker, and a shortened id can never be mistaken for a whole one. Picking a marker that the encoding can also emit would have reintroduced the ambiguity the dot rule just removed.

Injectivity now holds to the strength of a 64-bit digest rather than absolutely. That is a real weakening and worth saying plainly, against inputs that no real provider emits.

The six bytes that are not in the name

The last defect is my favourite, because the code was correct and the arithmetic was complete and it still could not write a file.

Two panels. Top: a horizontal stacked bar showing how 255 bytes of one filename are spent: 116 bytes of truncated title, one separator dot, 128 bytes of encoded item id, four bytes of extension, and a hatched six-byte segment reserved for the temporary name that does not appear in the final name at all. Bottom: final and temporary name lengths in bytes for a hostile title. Before the fix, a 500-character title derived a 255-byte final name and a 261-byte temporary name, which exceeds the 255-byte limit and is marked 'does not fit'. After the fix the same title derives 249 and 255 bytes, and titles of 230 and 300 characters derive 246/252 and 249/255, all at or under the limit.

The fetch does not write to the final name. It writes to a temporary file and renames it into place once the download finishes, so a crash cannot leave a truncated file that the skip check would then accept forever. The temporary name is the final name with a leading dot and a .part suffix, which is six bytes longer.

So the longest name the filesystem is ever asked to create is one nobody wrote down. Spending all 255 bytes on the final name meant that every title long enough to be truncated derived a final path that would write and a temporary path that would not: a 500-character title gave a 255-byte final name and a 261-byte temporary one, and the fetch died before writing a byte. The tool’s own hostile test row, a 500-character title, passed the byte-budget test and could never have been fetched, because the test exercised the derivation and the failure was in the composition.

The fix is to reserve those bytes in the budget, and to measure rather than type them:

_TEMP_PROBE = "x"

TEMP_DECORATION_BYTES = len(temp_path_for(Path(_TEMP_PROBE)).name.encode("utf-8")) - len(
    _TEMP_PROBE.encode("utf-8")
)

That is six against the current implementation. Writing 6 there would have been correct and would have quietly stopped being correct the day somebody changed the temporary naming, which is the kind of coupling that only shows up as a crash on the longest title anyone happens to try.

The budget then reads as one line of arithmetic, and it is asserted by a test rather than left in a comment to rot:

assert NAME_MAX_BYTES - TEMP_DECORATION_BYTES - 1 - ID_MAX_BYTES - len(EXTENSION) == 116

255 total, minus 6 for the decoration, minus 1 for the separator dot, minus 128 for the capped id, minus 4 for the extension, leaves the title 116 bytes. Every term measured or chosen deliberately. I have written before about how easily a number in a comment drifts away from the code beside it; a constant computed from the function it describes cannot.

One test that keeps the rest honest

The boundary test needs a percent-decoder to check that a shortened prefix did not split a token. The obvious decoder has a hole:

def _decode_percent(text: str) -> bytes:
    """Deliberately strict. A naive decoder that reads text[i+1:i+3] and calls
    int(...) accepts the split token `%2` silently, and the boundary test it
    backs then proves nothing."""

int("2", 16) is 2, not an error. A decoder that accepts %2 would pass every input the boundary test throws at it, and the test would be green and vacuous. So there is a test for the test helper, asserting that it raises on a%C and on a%. That is the only way I know to tell a passing test from a test that cannot fail, and it is the same hole I hit in a contract check that reported green while missing a case.

There is a smaller version of the same idea in the test data. The collision list ends with the NFC and NFD spellings of one grapheme, and the second test that needs that list imports it rather than retyping it, with a comment saying why: a hand copy renders the two entries identical and silently removes the case they exist for. Test fixtures can be destroyed by copy-paste in ways the tests cannot detect.

What to take from it

Ask of each field: is this for a human, or is this what tells two things apart? Sanitise the first. Never sanitise the second. When one field is both, split it into two fields, because you cannot have lossy and lossless behaviour out of one transform.

Prefer reversible to merely validated. “I checked it for bad characters” is an argument about the inputs you imagined. “This encoding can be decoded” is an argument about all of them, and it is shorter.

Compose the operations in your tests, not just each one. Every individual function here was correct on its own. The two failures that reached a crash were both in the seam: a name that derives and does not write, an id that writes and does not fetch. If a value passes through three functions before it reaches the operating system, the test that matters drives all three.

Compute your constants from the code they describe. Not because it is elegant, but because the alternative is a literal that is right today and silent when it stops being.

If you have a sixth collision in this family, I would genuinely like to hear it. The pattern I keep hitting is that each one looked like an edge case and turned out to be a whole class, and I do not believe I have found the last one.

References

  • RFC 3986, section 2.1 and 2.3 - the percent-encoded triplet, and the unreserved set ALPHA / DIGIT / "-" / "." / "_" / "~" that this encoding deliberately narrows.
  • Unicode Standard Annex #15: Normalization Forms - NFD as “Canonical Decomposition” and NFC as “Canonical Decomposition, followed by Canonical Composition”, and why “two equivalent strings will have precisely the same normalized form” is a property you want for a title and not for an identifier.
  • pathconf(3) - _PC_NAME_MAX, “the maximum length of a filename in the directory path or fd that the process is allowed to create”.
  • RFC 7693 - BLAKE2, whose b variant “produces digests of any size between 1 and 64 bytes”, which is where the 8-byte digest in the shortened id comes from.

Want the full background behind work like this?

Keep reading

Get the next update by email

Build-in-public updates and new posts, delivered as a digest. Double opt-in · no spam · unsubscribe anytime · handled by Buttondown.