Limit how much fetch and check read for a manifest or a file (closes #168)
check / check (push) Waiting to run
check / check (push) Waiting to run
NewManifestFromReader reads at most one byte past MaxManifestSize, a new constant of 258 MiB: the 256 MiB decompressed limit grown by zstd's worst case of 1/256, plus 1 MiB for the signature, the signing key and the other outer fields. It refuses a larger manifest. fetch, and check given a URL, stop downloading a manifest one byte past the same size and report it as too large; tests lower that size to keep their memory small. fetch stops reading a file one byte past its listed size, so a longer body ends in the size mismatch at once instead of filling the disk. docs/FORMAT.md states the limit and gives the decompressed limit as 256 MiB, the size the code uses. Model: opus-5-5
This commit was merged in pull request #172.
This commit is contained in:
+15
-6
@@ -361,7 +361,7 @@ func (mfa *CLIApp) fetchManifestOperation(
|
||||
firstDelay: firstRetryDelay,
|
||||
}
|
||||
|
||||
manifestData, files, err := fetchManifest(ctx, cmd, client, manifestURL)
|
||||
manifestData, files, err := mfa.fetchManifest(ctx, cmd, client, manifestURL)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
@@ -426,20 +426,22 @@ func (mfa *CLIApp) fetchManifestOperation(
|
||||
// that lists a file where fetch writes another or a mode outside 0777. It
|
||||
// returns the manifest as downloaded, to be saved once the files are in
|
||||
// place, and the files it lists.
|
||||
func fetchManifest(
|
||||
func (mfa *CLIApp) fetchManifest(
|
||||
ctx context.Context, cmd *cli.Command, client retryingClient, manifestURL string,
|
||||
) ([]byte, []*mfer.MFFilePath, error) {
|
||||
log.Infof("fetching manifest from %s", manifestURL)
|
||||
|
||||
// Read the whole manifest before parsing it, so that a connection
|
||||
// lost partway through is retried rather than reported as a bad
|
||||
// manifest.
|
||||
// manifest. Reading stops one byte past mfa.maxManifestSize, which is
|
||||
// enough to tell that the manifest is too large.
|
||||
var manifestData []byte
|
||||
|
||||
err := client.get(ctx, manifestURL, func(resp *http.Response) error {
|
||||
var readErr error
|
||||
|
||||
manifestData, readErr = io.ReadAll(resp.Body)
|
||||
manifestData, readErr = io.ReadAll(
|
||||
io.LimitReader(resp.Body, mfa.maxManifestSize+1))
|
||||
|
||||
return readErr
|
||||
})
|
||||
@@ -447,6 +449,11 @@ func fetchManifest(
|
||||
return nil, nil, fmt.Errorf("failed to fetch manifest: %w", err)
|
||||
}
|
||||
|
||||
if int64(len(manifestData)) > mfa.maxManifestSize {
|
||||
return nil, nil, fmt.Errorf("failed to fetch manifest: %w of %d bytes",
|
||||
errManifestTooLarge, mfa.maxManifestSize)
|
||||
}
|
||||
|
||||
// Parse manifest
|
||||
//nolint:contextcheck // mfer loads a manifest without a context
|
||||
manifest, err := mfer.NewManifestFromReader(bytes.NewReader(manifestData))
|
||||
@@ -908,8 +915,10 @@ func saveResponse(
|
||||
progress: progress,
|
||||
}
|
||||
|
||||
// Copy content while hashing and reporting progress
|
||||
written, copyErr := io.Copy(pw, resp.Body)
|
||||
// Copy content while hashing and reporting progress. One byte past
|
||||
// the listed size is enough for finishDownload to report a size
|
||||
// mismatch.
|
||||
written, copyErr := io.Copy(pw, io.LimitReader(resp.Body, expectedSize+1))
|
||||
|
||||
// Close file before checking errors (to flush writes)
|
||||
closeErr := out.Close()
|
||||
|
||||
Reference in New Issue
Block a user