diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ff5a5e..2eac0fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ ## Yara Rest Service Change Log +### [Unreleased] + +Stream uploads to disk instead of buffering them in memory; uploads over `MAX_UPLOAD_SIZE_MB` now get a 413 response + ### [v0.1.1] (12/05/2026) Send proper error responses in case a scan failed instead of an internal server error diff --git a/main.go b/main.go index 7a099b2..6ee5962 100644 --- a/main.go +++ b/main.go @@ -30,8 +30,11 @@ package main import ( "encoding/json" + "errors" + "fmt" "io" "log" + "mime/multipart" "net/http" "os" "strings" @@ -98,47 +101,33 @@ func handleScan(w http.ResponseWriter, r *http.Request) { startTime := time.Now() - // Parse multipart form with configured size limit - if err := r.ParseMultipartForm(config.MaxUploadSize); err != nil { - log.Printf("Failed to parse form: %v", err) - sendError(w, "Failed to parse form", http.StatusBadRequest) - return - } - - // Get uploaded file - file, header, err := r.FormFile("file") + tempPath, filename, err := saveUpload(w, r, scanTempDir) if err != nil { - log.Printf("No file provided: %v", err) - sendError(w, "No file provided", http.StatusBadRequest) + var tooLarge *http.MaxBytesError + switch { + case errors.As(err, &tooLarge): + log.Printf("Upload exceeds %d bytes", tooLarge.Limit) + sendError(w, "Upload too large", http.StatusRequestEntityTooLarge) + case errors.Is(err, errBadForm): + log.Printf("Failed to parse form: %v", err) + sendError(w, "Failed to parse form", http.StatusBadRequest) + case errors.Is(err, errNoFile): + log.Printf("No file provided: %v", err) + sendError(w, "No file provided", http.StatusBadRequest) + default: + log.Printf("Failed to save upload: %v", err) + sendError(w, "Failed to process upload", http.StatusInternalServerError) + } return } - defer file.Close() + defer os.Remove(tempPath) // Sanitize filename for logging (prevent log injection) - safeFilename := sanitizeFilename(header.Filename) - log.Printf("Received file: %s (%d bytes)", safeFilename, header.Size) - - // Create temp file for the upload - // Uses scanTempDir which has OpenShift-compatible permissions (GID 0) - tempFile, err := os.CreateTemp(scanTempDir, "yara-scan-*") - if err != nil { - log.Printf("Failed to create temp file: %v", err) - sendError(w, "Internal server error", http.StatusInternalServerError) - return - } - defer os.Remove(tempFile.Name()) - defer tempFile.Close() - - // Copy uploaded file to temp - if _, err := io.Copy(tempFile, file); err != nil { - log.Printf("Failed to save file: %v", err) - sendError(w, "Failed to process upload", http.StatusInternalServerError) - return - } - tempFile.Close() + safeFilename := sanitizeFilename(filename) + log.Printf("Received file: %s", safeFilename) // Scan the file - result, err := scanner.ScanFile(tempFile.Name()) + result, err := scanner.ScanFile(tempPath) if err != nil { log.Printf("Scan failed: %v", err) @@ -167,6 +156,83 @@ func handleScan(w http.ResponseWriter, r *http.Request) { sendResponse(w, response) } +var ( + errBadForm = errors.New("invalid multipart request") + errNoFile = errors.New("no file part in request") +) + +// saveUpload streams the "file" part of a multipart request into a new temp +// file in dir, so memory use stays constant regardless of upload size. +// The caller removes the returned path. +func saveUpload(w http.ResponseWriter, r *http.Request, dir string) (path, filename string, err error) { + r.Body = http.MaxBytesReader(w, r.Body, config.MaxUploadSize) + + mr, err := r.MultipartReader() + if err != nil { + return "", "", fmt.Errorf("%w: %v", errBadForm, err) + } + + for { + part, err := mr.NextPart() + if err == io.EOF { + return "", "", errNoFile + } + if err != nil { + return "", "", classifyReadError(err) + } + if part.FormName() == "file" { + return copyPart(part, dir) + } + } +} + +// copyPart writes the part to a new temp file in dir and returns its path. +func copyPart(part *multipart.Part, dir string) (string, string, error) { + defer part.Close() + + tempFile, err := os.CreateTemp(dir, "yara-scan-*") + if err != nil { + return "", "", err + } + if err := copyUpload(tempFile, part); err != nil { + tempFile.Close() + os.Remove(tempFile.Name()) + return "", "", err + } + if err := tempFile.Close(); err != nil { + os.Remove(tempFile.Name()) + return "", "", err + } + return tempFile.Name(), part.FileName(), nil +} + +// copyUpload copies src to dst. Only src failures are classified as request +// errors; a dst failure (disk full, I/O error) stays a plain server error. +func copyUpload(dst io.Writer, src io.Reader) error { + _, err := io.Copy(dst, readClassifier{src}) + return err +} + +type readClassifier struct{ io.Reader } + +func (r readClassifier) Read(p []byte) (int, error) { + n, err := r.Reader.Read(p) + if err != nil && err != io.EOF { + err = classifyReadError(err) + } + return n, err +} + +// classifyReadError keeps size-limit errors intact and marks any other +// body read failure as a malformed request. +func classifyReadError(err error) error { + var tooLarge *http.MaxBytesError + if errors.As(err, &tooLarge) { + return err + } + return fmt.Errorf("%w: %v", errBadForm, err) +} + // handleHealth handles GET /health requests func handleHealth(w http.ResponseWriter, r *http.Request) { response := map[string]interface{}{ diff --git a/main_test.go b/main_test.go index d6a71c6..f1a62da 100644 --- a/main_test.go +++ b/main_test.go @@ -3,12 +3,15 @@ package main import ( "bytes" "encoding/json" + "errors" "io" "mime/multipart" "net/http" "net/http/httptest" "os" + "runtime" "strings" + "syscall" "testing" "time" ) @@ -29,6 +32,11 @@ func init() { IdleTimeout: 120 * time.Second, } scanner = NewScanner(config) + + // handleScan writes uploads to scanTempDir, which only exists in the container image. + if err := os.MkdirAll(scanTempDir, 0o777); err != nil { + panic(err) + } } func TestHandleHealth(t *testing.T) { @@ -331,3 +339,119 @@ func TestIntegrationScanWithMatch(t *testing.T) { t.Logf("Scan result: %d matches, %d files, %dms", len(resp.Matches), resp.ScannedFiles, resp.ScanTimeMs) } + +func multipartRequest(t *testing.T, fields map[string]string, content io.Reader) *http.Request { + t.Helper() + pr, pw := io.Pipe() + writer := multipart.NewWriter(pw) + go func() { + for k, v := range fields { + writer.WriteField(k, v) + } + part, _ := writer.CreateFormFile("file", "big.vsix") + _, err := io.Copy(part, content) + writer.Close() + pw.CloseWithError(err) + }() + req := httptest.NewRequest(http.MethodPost, "/scan", pr) + req.Header.Set("Content-Type", writer.FormDataContentType()) + return req +} + +func TestSaveUploadStreamsToDisk(t *testing.T) { + const size = 64 << 20 + old := config.MaxUploadSize + config.MaxUploadSize = 2 * size + defer func() { config.MaxUploadSize = old }() + + dir := t.TempDir() + req := multipartRequest(t, map[string]string{"note": "x"}, io.LimitReader(zeroReader{}, size)) + + var before, after runtime.MemStats + runtime.ReadMemStats(&before) + path, filename, err := saveUpload(httptest.NewRecorder(), req, dir) + runtime.ReadMemStats(&after) + if err != nil { + t.Fatal(err) + } + + if filename != "big.vsix" { + t.Errorf("filename = %q, want big.vsix", filename) + } + if info, err := os.Stat(path); err != nil || info.Size() != size { + t.Errorf("saved file: info=%v err=%v, want %d bytes", info, err, size) + } + // Buffering the upload would allocate at least its full size. + if alloc := after.TotalAlloc - before.TotalAlloc; alloc > size/4 { + t.Errorf("allocated %d bytes for a %d byte upload", alloc, size) + } +} + +func TestSaveUploadTooLarge(t *testing.T) { + dir := t.TempDir() + req := multipartRequest(t, nil, io.LimitReader(zeroReader{}, config.MaxUploadSize+1)) + + _, _, err := saveUpload(httptest.NewRecorder(), req, dir) + + var tooLarge *http.MaxBytesError + if !errors.As(err, &tooLarge) { + t.Fatalf("err = %v, want *http.MaxBytesError", err) + } + if entries, _ := os.ReadDir(dir); len(entries) != 0 { + t.Errorf("temp file left behind: %v", entries) + } +} + +func TestHandleScanTooLarge(t *testing.T) { + req := multipartRequest(t, nil, io.LimitReader(zeroReader{}, config.MaxUploadSize+1)) + w := httptest.NewRecorder() + + handleScan(w, req) + + if w.Code != http.StatusRequestEntityTooLarge { + t.Errorf("status = %d, want %d", w.Code, http.StatusRequestEntityTooLarge) + } +} + +type zeroReader struct{} + +func (zeroReader) Read(p []byte) (int, error) { + clear(p) + return len(p), nil +} + +type errWriter struct{ err error } + +func (w errWriter) Write([]byte) (int, error) { return 0, w.err } + +type errReader struct{ err error } + +func (r errReader) Read([]byte) (int, error) { return 0, r.err } + +func TestCopyUploadWriteErrorIsNotBadRequest(t *testing.T) { + err := copyUpload(errWriter{syscall.ENOSPC}, strings.NewReader("data")) + + if !errors.Is(err, syscall.ENOSPC) { + t.Fatalf("err = %v, want ENOSPC", err) + } + if errors.Is(err, errBadForm) { + t.Errorf("write failure classified as bad request: %v", err) + } +} + +func TestCopyUploadReadErrorIsBadRequest(t *testing.T) { + err := copyUpload(io.Discard, errReader{io.ErrUnexpectedEOF}) + + if !errors.Is(err, errBadForm) { + t.Errorf("err = %v, want errBadForm", err) + } +} + +func TestCopyUploadKeepsSizeLimitError(t *testing.T) { + err := copyUpload(io.Discard, errReader{&http.MaxBytesError{Limit: 1}}) + + var tooLarge *http.MaxBytesError + if !errors.As(err, &tooLarge) || errors.Is(err, errBadForm) { + t.Errorf("err = %v, want unwrapped *http.MaxBytesError", err) + } +}