Repository navigation
Close the small gaps a review of the library found - #127
Merged
Merged
Conversation
- A symlink to nothing at the output path made the JVM fail with a bare "no such file": the link exists, so it was canonicalized, which has nothing to resolve to. It is now replaced like a missing file, as the comment beside it always said. - A text or column region that names the range the last one named is served from that read rather than reading the source again, which was the cheapest way for a patch to make a reader work. - The encoder closes the whole-source scanner it built, and a group's scanners even when a child fails, for an algorithm that holds something in one; the bundled one holds nothing. - Pairing by content kept the first child with a key but also carried a line that could never run; it now says what it does. - A native replace carried setuid, setgid and sticky bits onto the new file where the JVM carried the nine permission bits. Both now carry nine: a kernel clears the first two on the write anyway, and a new file should not inherit a privilege from the one it replaces. - A source of exactly Int.MAX_VALUE bytes was accepted and then failed to allocate; the limit is the largest array a JVM holds, as the whole-file read already used. - A directory given as a source opened on Linux and macOS and failed on its first read, partway through; it is refused when opened. - A quoted name's octal escape past \377 wrapped to a byte; it is refused, as git refuses it.
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Eight low-severity findings from a robustness review, none of which changes a patch's bytes.
KiffFiles.replace): on JVM, okio reports the link as existing andcanonicalizethen throws a bareFileNotFoundException. The link is now replaced like a missing file, as the comment said. New testaLinkToNothingIsReplacedLikeAMissingFile.PatchPayload.Restore.read): a TEXT or COLUMNS region naming the same range as the previous one reuses that read instead of reading the source again. A crafted patch could name the whole source from thousands of regions; the bound for different ranges is the one FORMAT.md states and is unchanged.ContainerEncoder): the whole-source scanner is closed after the encode, and a group's scanners are closed infinally. Matters only for a caller'sDeltaAlgorithmthat holds resources.Pairing.byContent: the un-pair branch could never run; the map now plainly keeps the first child per key, which is what it did.posixMain/FileAttributes.kt): native copied the setuid, setgid and sticky bits onto a replaced file while JVM copied only the nine permission bits. Both now copy nine.Materialize.requireIndexable): a source of exactlyInt.MAX_VALUEbytes passed the check and died withOutOfMemoryError; the sharedLARGEST_ARRAYlimit is used.fileSource): refused on open withUnsupportedInputrather than failing on the first read on Linux/macOS. New JVM test.UnifiedDiff.nameOf):\777wrapped silently to a byte; it is refused as git does.Validation:
./gradlew jvmTest :library:checkKotlinAbi :library:macosArm64Testpass. The ABI dump is unchanged.