diff --git a/.golangci.yml b/.golangci.yml index 9087d1a3d..ceaff8e2b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -76,6 +76,9 @@ linters: # Don't necessarily rewrite !(foo || bar) to !foo && !bar - linters: [staticcheck] text: QF1001 + # Don't necessarily pass the context just for the sake of the logger + - linters: [contextcheck] + text: "->log`" settings: sloglint: context: "scope" diff --git a/lib/model/folder_sendrecv.go b/lib/model/folder_sendrecv.go index 26f1c0fbc..7d7238b19 100644 --- a/lib/model/folder_sendrecv.go +++ b/lib/model/folder_sendrecv.go @@ -1558,10 +1558,21 @@ func (f *sendReceiveFolder) pullBlock(ctx context.Context, state pullBlockState, return } - if !f.DisableSparseFiles && state.reused == 0 && state.block.IsEmpty() { + if state.block.IsEmpty() { // There is no need to request a block of all zeroes. Pretend we // requested it and handled it correctly. - state.pullDone(state.block) + if state.reused != 0 || f.DisableSparseFiles { + // We are reusing a file (contents apparently weren't all-zeroes + // previously), or sparse files are disabled, so we need to + // actually write the block. + zeroes := make([]byte, state.block.Size) + err = f.limitedWriteAt(ctx, fd, zeroes, state.block.Offset) + } + if err != nil { + state.fail(fmt.Errorf("save: %w", err)) + } else { + state.pullDone(state.block) + } out <- state.sharedPullerState return } @@ -1607,11 +1618,10 @@ loop: } // Verify that the received block matches the desired hash, if not - // try pulling it from another device. - // For receive-only folders, the hash is not SHA256 as it's an - // encrypted hash token. In that case we can't verify the block - // integrity so we'll take it on trust. (The other side can and - // will verify.) + // try pulling it from another device. For receive-encrypted + // folders, the hash is not SHA256 as it's an encrypted hash token. + // In that case we can't verify the block integrity so we'll take it + // on trust. (The other side can and will verify.) if f.Type != config.FolderTypeReceiveEncrypted { lastError = f.verifyBuffer(buf, state.block) } diff --git a/lib/model/folder_sendrecv_test.go b/lib/model/folder_sendrecv_test.go index a84de43be..8bd56b088 100644 --- a/lib/model/folder_sendrecv_test.go +++ b/lib/model/folder_sendrecv_test.go @@ -917,6 +917,88 @@ func TestPullCtxCancel(t *testing.T) { } } +// TestPullEmptyBlock exercises pullBlock for blocks of all zeroes. +// pullBlock distinguishes three cases: +// +// - Sparse files enabled, reused == 0: nothing to write, the offset +// reads as zero anyway because the temp file is sparse-allocated. +// - reused != 0: the temp file is being reused and may already hold +// non-zero data at this offset, so zeroes must be written explicitly. +// - DisableSparseFiles: cannot rely on a sparse hole reading as zero, +// so zeroes must be written explicitly. +func TestPullEmptyBlock(t *testing.T) { + const blockSize = protocol.MinBlockSize + emptyBlock := protocol.BlockInfo{ + Offset: 0, + Size: blockSize, + Hash: blocks[0].Hash, // sha256 of a 128 KiB all-zeroes block + } + if !emptyBlock.IsEmpty() { + t.Fatal("test setup: block not recognised as empty") + } + + cases := []struct { + name string + reused []int + disableSparse bool + preFillTempFile bool + }{ + {name: "sparse-clean"}, + {name: "reused-temp-file", reused: []int{1}, preFillTempFile: true}, + {name: "disable-sparse-files", disableSparse: true}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, f := setupSendReceiveFolder(t) + f.folder.FolderConfiguration.DisableSparseFiles = tc.disableSparse + + file := protocol.FileInfo{ + Name: "file", + Size: int64(blockSize), + Permissions: 0o644, + Blocks: []protocol.BlockInfo{emptyBlock}, + } + tempName := fs.TempName(file.Name) + + // Pre-fill the temp file with non-zero data so that a missing + // overwrite would be detectable. + if tc.preFillTempFile { + writeFile(t, f.Filesystem(), tempName, bytes.Repeat([]byte{0xff}, blockSize)) + } + + state := pullBlockState{ + sharedPullerState: newSharedPullerState(file, f.Filesystem(), f.folderID, tempName, file.Blocks, tc.reused, false, false, protocol.FileInfo{}, !tc.disableSparse, false), + block: emptyBlock, + } + + out := make(chan *sharedPullerState, 1) + f.pullBlock(t.Context(), state, out) + + s := <-out + if err := s.failed(); err != nil { + t.Fatalf("pullBlock failed: %v", err) + } + cleanupSharedPullerState(s) + + fd, err := f.Filesystem().Open(tempName) + must(t, err) + defer fd.Close() + data, err := io.ReadAll(fd) + must(t, err) + + if len(data) != blockSize { + t.Fatalf("temp file too short: got %d bytes, want at least %d", len(data), emptyBlock.Offset+int64(blockSize)) + } + for i, b := range data { + if b != 0 { + t.Fatalf("byte %d: got %#x, want 0", i, b) + } + } + }) + } +} + func TestPullDeleteUnscannedDir(t *testing.T) { _, f := setupSendReceiveFolder(t) ffs := f.Filesystem()