lore-storage: Make read_into respect the byte range for single-fragment reads - #37
Closed
itsfuad wants to merge 1 commit into
Closed
lore-storage: Make read_into respect the byte range for single-fragment reads#37itsfuad wants to merge 1 commit into
itsfuad wants to merge 1 commit into
Conversation
rajpratham1
approved these changes
Jun 20, 2026
rajpratham1
left a comment
rajpratham1
left a comment
There was a problem hiding this comment.
This is a focused bug fix that addresses a clear inconsistency in read_into. The change is minimal, aligns the single-fragment paths with the existing fragmented implementation and read(), and includes a regression test that reproduces the original issue. I don't see any blocking concerns.
mealicecooper
approved these changes
Jun 20, 2026
mjansson
changed the title
Collaborator
|
Looks good - make sure you follow the guidelines at https://github.com/EpicGames/lore/blob/main/CONTRIBUTING.md#dco-sign-off and do the DCO signoff on the commits to let us take this in. |
Contributor
Author
@mjansson Done. Review please. |
mjansson
approved these changes
Jun 22, 2026
Fixes EpicGames#25. The two single-fragment branches (compressed and uncompressed) in read_into now apply buffer.slice(range) before copying into the output slice, matching what read() already does. Added a regression test that stores a 100-byte fragment and reads bytes 10..50 into a 40-byte buffer. Signed-off-by: Fuad Hasan <fuad.cs22@gmail.com>
Contributor
Author
|
@mjansson Fixed clippy issues in workflow run. |
Collaborator
|
We'll get this merged once the intake and attribution process is in place, ideally early next week. |
epic-lore-bot
Bot
added
imported
|
Imported as Lore CR-255. |
epic-lore-bot Bot
pushed a commit
that referenced
this pull request
Jul 29, 2026
…nt reads **What:** Make `read_into` respect the byte range for single-fragment reads in `lore-storage`. **Why:** `read_into` accepts an optional byte `range`, but both single-fragment branches (compressed and uncompressed) were comparing `slice.len()` against the full fragment payload instead of the requested range. Any partial read on a file ≤ 256 KiB (the `FRAGMENT_SIZE_THRESHOLD`) would fail with `unexpected size: slice 40 vs buffer 100`. The fragmented branch and the sibling `read()` both handle this correctly, only `read_into`'s single-fragment paths were missing it. Fixes #25 **How:** Added one line to each single-fragment branch to slice the buffer to the requested range before the existing size check and copy: ```rust // compressed let decompressed = decompressed.freeze().slice(range); // uncompressed let buffer = buffer.slice(range); ``` **Testing:** Added a regression test that stores a 100-byte fragment and reads bytes 10..50 into a 40-byte buffer, reproducing the original failure. ``` Imported-PR: #37 Imported-From: e7f0159 Imported-Base: c920a7f Imported-Merge: d7ec30c Imported-Author: Fuad Hasan (itsfuad) Signed-off-by: Fuad Hasan <fuad.cs22@gmail.com> GH-URL: #37 ``` Lore-RevId: 369 Lore-Signature: ff073a1673a5c82adf7670ab0e3c09c4758c93364ee5d9ab6b9ff64fdcd5a64b
|
Closed by mirrored commit e4202e7. |
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
4 participants
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.
What:
Make
read_intorespect the byte range for single-fragment reads inlore-storage.Why:
read_intoaccepts an optional byterange, but both single-fragment branches (compressed and uncompressed) were comparingslice.len()against the full fragment payload instead of the requested range. Any partial read on a file ≤ 256 KiB (theFRAGMENT_SIZE_THRESHOLD) would fail withunexpected size: slice 40 vs buffer 100. The fragmented branch and the siblingread()both handle this correctly, onlyread_into's single-fragment paths were missing it.Fixes #25
How:
Added one line to each single-fragment branch to slice the buffer to the requested range before the existing size check and copy:
Testing:
Added a regression test that stores a 100-byte fragment and reads bytes 10..50 into a 40-byte buffer, reproducing the original failure.