error on out of space

api/files.go

@@ -114,34 +114,69 @@ func apiFilesUpload(c *gin.Context) {
 	}
 
 	stored := []string{}
+	openFiles := []*os.File{}
 
+	// Write all files first
 	for _, files := range c.Request.MultipartForm.File {
 		for _, file := range files {
 			src, err := file.Open()
 			if err != nil {
+				// Close any files we've opened
+				for _, f := range openFiles {
+					_ = f.Close()
+				}
 				AbortWithJSONError(c, 500, err)
 				return
 			}
-			defer func() { _ = src.Close() }()
 
 			destPath := filepath.Join(path, filepath.Base(file.Filename))
 			dst, err := os.Create(destPath)
 			if err != nil {
+				_ = src.Close()
+				// Close any files we've opened
+				for _, f := range openFiles {
+					_ = f.Close()
+				}
 				AbortWithJSONError(c, 500, err)
 				return
 			}
-			defer func() { _ = dst.Close() }()
 
 			_, err = io.Copy(dst, src)
+			_ = src.Close()
 			if err != nil {
+				_ = dst.Close()
+				// Close any files we've opened
+				for _, f := range openFiles {
+					_ = f.Close()
+				}
 				AbortWithJSONError(c, 500, err)
 				return
 			}
 
+			// Keep file open for batch sync
+			openFiles = append(openFiles, dst)
 			stored = append(stored, filepath.Join(c.Params.ByName("dir"), filepath.Base(file.Filename)))
 		}
 	}
 
+	// Sync all files at once to catch ENOSPC errors
+	for i, dst := range openFiles {
+		err := dst.Sync()
+		if err != nil {
+			// Close all files
+			for _, f := range openFiles {
+				_ = f.Close()
+			}
+			AbortWithJSONError(c, 500, fmt.Errorf("error syncing file %s: %s", stored[i], err))
+			return
+		}
+	}
+
+	// Close all files
+	for _, dst := range openFiles {
+		_ = dst.Close()
+	}
+
 	apiFilesUploadedCounter.WithLabelValues(c.Params.ByName("dir")).Inc()
 	c.JSON(200, stored)
 }

api/files_test.go

@@ -0,0 +1,187 @@
+package api
+
+import (
+	"bytes"
+	"encoding/json"
+	"io"
+	"mime/multipart"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"path/filepath"
+
+	"github.com/aptly-dev/aptly/aptly"
+	ctx "github.com/aptly-dev/aptly/context"
+	"github.com/gin-gonic/gin"
+	"github.com/smira/flag"
+
+	. "gopkg.in/check.v1"
+)
+
+type FilesUploadDiskFullSuite struct {
+	aptlyContext *ctx.AptlyContext
+	flags        *flag.FlagSet
+	configFile   *os.File
+	router       http.Handler
+}
+
+var _ = Suite(&FilesUploadDiskFullSuite{})
+
+func (s *FilesUploadDiskFullSuite) SetUpTest(c *C) {
+	aptly.Version = "testVersion"
+
+	// Create temporary config
+	file, err := os.CreateTemp("", "aptly")
+	c.Assert(err, IsNil)
+	s.configFile = file
+
+	jsonString, err := json.Marshal(gin.H{
+		"architectures": []string{},
+		"rootDir":       c.MkDir(),
+	})
+	c.Assert(err, IsNil)
+	_, err = file.Write(jsonString)
+	c.Assert(err, IsNil)
+	_ = file.Close()
+
+	// Setup flags and context
+	flags := flag.NewFlagSet("fakeFlags", flag.ContinueOnError)
+	flags.Bool("no-lock", false, "dummy")
+	flags.Int("db-open-attempts", 3, "dummy")
+	flags.String("config", s.configFile.Name(), "dummy")
+	flags.String("architectures", "", "dummy")
+	s.flags = flags
+
+	aptlyContext, err := ctx.NewContext(s.flags)
+	c.Assert(err, IsNil)
+
+	s.aptlyContext = aptlyContext
+	s.router = Router(aptlyContext)
+	context = aptlyContext // set global context
+}
+
+func (s *FilesUploadDiskFullSuite) TearDownTest(c *C) {
+	if s.configFile != nil {
+		_ = os.Remove(s.configFile.Name())
+	}
+	if s.aptlyContext != nil {
+		s.aptlyContext.Shutdown()
+	}
+}
+
+// TestUploadSuccessWithSync verifies that file uploads succeed when there's space
+// and that the Sync() call is made (by verifying the file is complete)
+func (s *FilesUploadDiskFullSuite) TestUploadSuccessWithSync(c *C) {
+	// Create a test file to upload
+	testContent := []byte("test file content for upload")
+
+	// Create multipart form
+	body := &bytes.Buffer{}
+	writer := multipart.NewWriter(body)
+
+	part, err := writer.CreateFormFile("file", "testfile.txt")
+	c.Assert(err, IsNil)
+
+	_, err = part.Write(testContent)
+	c.Assert(err, IsNil)
+
+	err = writer.Close()
+	c.Assert(err, IsNil)
+
+	// Create request
+	req, err := http.NewRequest("POST", "/api/files/testdir", body)
+	c.Assert(err, IsNil)
+	req.Header.Set("Content-Type", writer.FormDataContentType())
+
+	// Create response recorder
+	w := httptest.NewRecorder()
+
+	// Call handler
+	s.router.ServeHTTP(w, req)
+
+	// Check response
+	c.Assert(w.Code, Equals, 200)
+
+	// Verify file was written and synced
+	uploadedFile := filepath.Join(s.aptlyContext.Config().GetRootDir(), "upload", "testdir", "testfile.txt")
+	content, err := os.ReadFile(uploadedFile)
+	c.Assert(err, IsNil)
+	c.Check(content, DeepEquals, testContent)
+}
+
+// TestUploadVerifiesFileIntegrity ensures uploaded files are complete
+func (s *FilesUploadDiskFullSuite) TestUploadVerifiesFileIntegrity(c *C) {
+	// Create larger test file
+	testContent := bytes.Repeat([]byte("A"), 10000)
+
+	body := &bytes.Buffer{}
+	writer := multipart.NewWriter(body)
+
+	part, err := writer.CreateFormFile("file", "largefile.bin")
+	c.Assert(err, IsNil)
+
+	_, err = io.Copy(part, bytes.NewReader(testContent))
+	c.Assert(err, IsNil)
+
+	err = writer.Close()
+	c.Assert(err, IsNil)
+
+	req, err := http.NewRequest("POST", "/api/files/testdir2", body)
+	c.Assert(err, IsNil)
+	req.Header.Set("Content-Type", writer.FormDataContentType())
+
+	w := httptest.NewRecorder()
+	s.router.ServeHTTP(w, req)
+
+	c.Assert(w.Code, Equals, 200)
+
+	// Verify complete file was written
+	uploadedFile := filepath.Join(s.aptlyContext.Config().GetRootDir(), "upload", "testdir2", "largefile.bin")
+	content, err := os.ReadFile(uploadedFile)
+	c.Assert(err, IsNil)
+	c.Check(len(content), Equals, len(testContent))
+	c.Check(content, DeepEquals, testContent)
+}
+
+// TestUploadMultipleFilesWithBatchSync tests that multiple files are synced in batch
+func (s *FilesUploadDiskFullSuite) TestUploadMultipleFilesWithBatchSync(c *C) {
+	// Create multiple test files with different content
+	testFiles := map[string][]byte{
+		"file1.txt": []byte("content of file 1"),
+		"file2.txt": bytes.Repeat([]byte("B"), 5000),
+		"file3.deb": []byte("debian package content"),
+	}
+
+	body := &bytes.Buffer{}
+	writer := multipart.NewWriter(body)
+
+	// Add all files to multipart form
+	for filename, content := range testFiles {
+		part, err := writer.CreateFormFile("file", filename)
+		c.Assert(err, IsNil)
+		_, err = part.Write(content)
+		c.Assert(err, IsNil)
+	}
+
+	err := writer.Close()
+	c.Assert(err, IsNil)
+
+	req, err := http.NewRequest("POST", "/api/files/multitest", body)
+	c.Assert(err, IsNil)
+	req.Header.Set("Content-Type", writer.FormDataContentType())
+
+	w := httptest.NewRecorder()
+	s.router.ServeHTTP(w, req)
+
+	// Verify response
+	c.Assert(w.Code, Equals, 200)
+
+	// Verify all files were written and synced correctly
+	uploadDir := filepath.Join(s.aptlyContext.Config().GetRootDir(), "upload", "multitest")
+	for filename, expectedContent := range testFiles {
+		uploadedFile := filepath.Join(uploadDir, filename)
+		content, err := os.ReadFile(uploadedFile)
+		c.Assert(err, IsNil, Commentf("Failed to read %s", filename))
+		c.Check(content, DeepEquals, expectedContent, Commentf("Content mismatch for %s", filename))
+	}
+}

files/public.go

@@ -119,7 +119,17 @@ func (storage *PublishedStorage) PutFile(path string, sourceFilename string) err
 	}()
 
 	_, err = io.Copy(f, source)
+	if err != nil {
 		return err
+	}
+
+	// Sync to ensure all data is written to disk and catch ENOSPC errors
+	err = f.Sync()
+	if err != nil {
+		return fmt.Errorf("error syncing file %s: %s", path, err)
+	}
+
+	return nil
 }
 
 // Remove removes single file under public path
@@ -268,6 +278,13 @@ func (storage *PublishedStorage) LinkFromPool(publishedPrefix, publishedRelPath,
 			return err
 		}
 
+		// Sync to ensure all data is written to disk and catch ENOSPC errors
+		err = dst.Sync()
+		if err != nil {
+			_ = dst.Close()
+			return fmt.Errorf("error syncing file %s: %s", destinationPath, err)
+		}
+
 		err = dst.Close()
 	} else if storage.linkMethod == LinkMethodSymLink {
 		err = localSourcePool.Symlink(sourcePath, destinationPath)

files/public_test.go

@@ -2,7 +2,10 @@ package files
 
 import (
 	"os"
+	"os/exec"
 	"path/filepath"
+	"runtime"
+	"strings"
 	"syscall"
 
 	"github.com/aptly-dev/aptly/aptly"
@@ -337,3 +340,184 @@ func (s *PublishedStorageSuite) TestRootRemove(c *C) {
 	dirStorage := NewPublishedStorage(pwd, "", "")
 	c.Assert(func() { _ = dirStorage.RemoveDirs("", nil) }, PanicMatches, "trying to remove the root directory")
 }
+
+// Disk full error handling tests
+
+type DiskFullSuite struct {
+	root string
+}
+
+var _ = Suite(&DiskFullSuite{})
+
+func (s *DiskFullSuite) SetUpTest(c *C) {
+	// Only run on Linux where we can create loopback filesystems
+	if runtime.GOOS != "linux" {
+		c.Skip("disk full tests only run on Linux")
+	}
+
+	// Check if running as root or with sudo capabilities
+	if os.Geteuid() != 0 {
+		c.Skip("disk full tests require root privileges")
+	}
+
+	s.root = c.MkDir()
+}
+
+// TestPutFileOutOfSpace tests that PutFile properly reports disk full errors
+func (s *DiskFullSuite) TestPutFileOutOfSpace(c *C) {
+	mountPoint := filepath.Join(s.root, "smallfs")
+	err := os.MkdirAll(mountPoint, 0777)
+	c.Assert(err, IsNil)
+
+	// Create a very small filesystem (1MB)
+	fsImage := filepath.Join(s.root, "small.img")
+
+	// Create 1MB sparse file
+	cmd := exec.Command("dd", "if=/dev/zero", "of="+fsImage, "bs=1M", "count=1")
+	err = cmd.Run()
+	c.Assert(err, IsNil)
+
+	// Format as ext4
+	cmd = exec.Command("mkfs.ext4", "-F", fsImage)
+	err = cmd.Run()
+	c.Assert(err, IsNil)
+
+	// Mount the filesystem
+	cmd = exec.Command("mount", "-o", "loop", fsImage, mountPoint)
+	err = cmd.Run()
+	c.Assert(err, IsNil)
+	defer func() {
+		_ = exec.Command("umount", mountPoint).Run()
+	}()
+
+	// Create storage on the small filesystem
+	storage := NewPublishedStorage(mountPoint, "", "")
+
+	// Create a large source file that won't fit (2MB)
+	largeFile := filepath.Join(s.root, "largefile")
+	cmd = exec.Command("dd", "if=/dev/zero", "of="+largeFile, "bs=1M", "count=2")
+	err = cmd.Run()
+	c.Assert(err, IsNil)
+
+	// Try to put the large file - should fail with out of space error
+	err = storage.PutFile("testfile", largeFile)
+	c.Assert(err, NotNil)
+	c.Check(strings.Contains(err.Error(), "no space left on device") ||
+		strings.Contains(err.Error(), "sync"), Equals, true,
+		Commentf("Expected disk full error, got: %v", err))
+}
+
+// TestLinkFromPoolCopyOutOfSpace tests that LinkFromPool with copy mode properly reports disk full errors
+func (s *DiskFullSuite) TestLinkFromPoolCopyOutOfSpace(c *C) {
+	mountPoint := filepath.Join(s.root, "smallfs")
+	err := os.MkdirAll(mountPoint, 0777)
+	c.Assert(err, IsNil)
+
+	// Create a very small filesystem (1MB)
+	fsImage := filepath.Join(s.root, "small.img")
+
+	cmd := exec.Command("dd", "if=/dev/zero", "of="+fsImage, "bs=1M", "count=1")
+	err = cmd.Run()
+	c.Assert(err, IsNil)
+
+	cmd = exec.Command("mkfs.ext4", "-F", fsImage)
+	err = cmd.Run()
+	c.Assert(err, IsNil)
+
+	cmd = exec.Command("mount", "-o", "loop", fsImage, mountPoint)
+	err = cmd.Run()
+	c.Assert(err, IsNil)
+	defer func() {
+		_ = exec.Command("umount", mountPoint).Run()
+	}()
+
+	// Create storage on the small filesystem using copy mode
+	storage := NewPublishedStorage(mountPoint, "copy", "")
+
+	// Create a normal pool in a different location
+	poolPath := filepath.Join(s.root, "pool")
+	pool := NewPackagePool(poolPath, false)
+	cs := NewMockChecksumStorage()
+
+	// Create a large package file (2MB) in the pool
+	largeFile := filepath.Join(s.root, "package.deb")
+	cmd = exec.Command("dd", "if=/dev/zero", "of="+largeFile, "bs=1M", "count=2")
+	err = cmd.Run()
+	c.Assert(err, IsNil)
+
+	sourceChecksum, err := utils.ChecksumsForFile(largeFile)
+	c.Assert(err, IsNil)
+
+	srcPoolPath, err := pool.Import(largeFile, "package.deb",
+		&utils.ChecksumInfo{MD5: "d41d8cd98f00b204e9800998ecf8427e"}, false, cs)
+	c.Assert(err, IsNil)
+
+	// Try to link from pool - should fail with out of space error
+	err = storage.LinkFromPool("", "pool/main/p/package", "package.deb",
+		pool, srcPoolPath, sourceChecksum, false)
+	c.Assert(err, NotNil)
+	c.Check(strings.Contains(err.Error(), "no space left on device") ||
+		strings.Contains(err.Error(), "sync"), Equals, true,
+		Commentf("Expected disk full error, got: %v", err))
+}
+
+// Alternative simpler test that doesn't require root
+type DiskFullNoRootSuite struct {
+	root string
+}
+
+var _ = Suite(&DiskFullNoRootSuite{})
+
+func (s *DiskFullNoRootSuite) SetUpTest(c *C) {
+	s.root = c.MkDir()
+}
+
+// This test verifies Sync() is called by checking it doesn't panic
+// The actual disk full behavior is harder to test without root
+func (s *DiskFullNoRootSuite) TestSyncIsCalled(c *C) {
+	storage := NewPublishedStorage(s.root, "", "")
+
+	// Create a small test file
+	sourceFile := filepath.Join(s.root, "source.txt")
+	err := os.WriteFile(sourceFile, []byte("test content"), 0644)
+	c.Assert(err, IsNil)
+
+	// PutFile should succeed with normal disk space
+	err = storage.PutFile("dest.txt", sourceFile)
+	c.Assert(err, IsNil)
+
+	// Verify file was written
+	content, err := os.ReadFile(filepath.Join(s.root, "dest.txt"))
+	c.Assert(err, IsNil)
+	c.Check(string(content), Equals, "test content")
+}
+
+func (s *DiskFullNoRootSuite) TestLinkFromPoolCopySyncIsCalled(c *C) {
+	storage := NewPublishedStorage(s.root, "copy", "")
+	poolPath := filepath.Join(s.root, "pool")
+	pool := NewPackagePool(poolPath, false)
+	cs := NewMockChecksumStorage()
+
+	// Create a test package file
+	pkgFile := filepath.Join(s.root, "package.deb")
+	err := os.WriteFile(pkgFile, []byte("package content"), 0644)
+	c.Assert(err, IsNil)
+
+	sourceChecksum, err := utils.ChecksumsForFile(pkgFile)
+	c.Assert(err, IsNil)
+
+	srcPoolPath, err := pool.Import(pkgFile, "package.deb",
+		&utils.ChecksumInfo{MD5: "d41d8cd98f00b204e9800998ecf8427e"}, false, cs)
+	c.Assert(err, IsNil)
+
+	// LinkFromPool with copy should succeed with normal disk space
+	err = storage.LinkFromPool("", "pool/main/p/package", "package.deb",
+		pool, srcPoolPath, sourceChecksum, false)
+	c.Assert(err, IsNil)
+
+	// Verify file was written
+	destPath := filepath.Join(s.root, "pool/main/p/package/package.deb")
+	content, err := os.ReadFile(destPath)
+	c.Assert(err, IsNil)
+	c.Check(string(content), Equals, "package content")
+}