From 383c2e3b4144dd35fcc63b4800181fcf551d6106 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Sun, 4 Jan 2026 13:55:33 -0800 Subject: [PATCH] fix: handle range requests on empty objects (size=0) (#7963) * fix: handle range requests on empty objects (size=0) Range requests on empty objects were incorrectly being rejected with: 'invalid range start for ...: 0 >= 0' The validation logic used 'startOffset >= totalSize' which failed when both were 0, incorrectly rejecting valid range requests like bytes=0-1535 on 0-byte files. Fix: Added special case handling before validation to properly return 416 Range Not Satisfiable for any range request on an empty object, per RFC 7233. Fixed at two locations (lines 873 and 1154) in s3api_object_handlers.go * refactor: return 404 for directory objects, not 416 Per S3 semantics, GET requests on directory paths (without trailing "/") should return 404 Not Found, not try to serve them as objects. Updated fix to: 1. Check if entry.IsDirectory and return 404 (S3-compliant) 2. Only return 416 for true empty files (size=0, not directory) This matches AWS S3 behavior where directories don't exist as objects unless they're explicit directory markers ending with "/". * reduce repeated info * refactor: move directory check before range branching This ensures that any Range header (including suffix ranges like bytes=-N) on a directory path (without trailing slash) returns 404 (ErrNoSuchKey) instead of potentially returning 416 or attempting to serve as an object. Applied to both streamFromVolumeServers and streamFromVolumeServersWithSSE. * refactoring --- weed/s3api/s3api_object_handlers.go | 246 +++++++++++----------------- 1 file changed, 98 insertions(+), 148 deletions(-) diff --git a/weed/s3api/s3api_object_handlers.go b/weed/s3api/s3api_object_handlers.go index 7f4c0e932..d7df58150 100644 --- a/weed/s3api/s3api_object_handlers.go +++ b/weed/s3api/s3api_object_handlers.go @@ -138,6 +138,95 @@ func adjustRangeForPart(partStartOffset, partEndOffset int64, clientRangeHeader return adjustedStart, adjustedEnd, nil } +// parseAndValidateRange parses the Range header and validates it against the object size. +// It also handles SeaweedFS-specific directory object checks. +// Returns: +// - offset: the absolute start offset in the object +// - size: the number of bytes to read +// - isRangeRequest: true if the client requested a range +// - err: nil on success, StreamError on failure (wraps S3 error response) +func (s3a *S3ApiServer) parseAndValidateRange(w http.ResponseWriter, r *http.Request, entry *filer_pb.Entry, totalSize int64, bucket, object string) (offset, size int64, isRangeRequest bool, err *StreamError) { + rangeHeader := r.Header.Get("Range") + if rangeHeader == "" || !strings.HasPrefix(rangeHeader, "bytes=") { + return 0, totalSize, false, nil + } + + rangeSpec := rangeHeader[6:] + parts := strings.Split(rangeSpec, "-") + if len(parts) != 2 { + return 0, totalSize, false, nil + } + + // S3 semantics: directories (without trailing "/") should return 404 + if entry.IsDirectory { + s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchKey) + return 0, 0, false, newStreamErrorWithResponse(fmt.Errorf("directory object %s/%s cannot be retrieved", bucket, object)) + } + + var startOffset, endOffset int64 + if parts[0] == "" && parts[1] != "" { + // Suffix range: bytes=-N (last N bytes) + if suffixLen, err := strconv.ParseInt(parts[1], 10, 64); err == nil { + // RFC 7233: suffix range on empty object or zero-length suffix is unsatisfiable + if totalSize == 0 || suffixLen <= 0 { + w.Header().Set("Content-Range", fmt.Sprintf("bytes */%d", totalSize)) + s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRange) + return 0, 0, false, newStreamErrorWithResponse(fmt.Errorf("invalid suffix range for empty object")) + } + if suffixLen > totalSize { + suffixLen = totalSize + } + startOffset = totalSize - suffixLen + endOffset = totalSize - 1 + } else { + w.Header().Set("Content-Range", fmt.Sprintf("bytes */%d", totalSize)) + s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRange) + return 0, 0, false, newStreamErrorWithResponse(fmt.Errorf("invalid suffix range")) + } + } else { + // Regular range or open-ended range + startOffset = 0 + endOffset = totalSize - 1 + + if parts[0] != "" { + if parsed, err := strconv.ParseInt(parts[0], 10, 64); err == nil { + startOffset = parsed + } + } + if parts[1] != "" { + if parsed, err := strconv.ParseInt(parts[1], 10, 64); err == nil { + endOffset = parsed + } + } + + // Special case: range requests on empty files should return 416 + if totalSize == 0 { + w.Header().Set("Content-Range", "bytes */0") + s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRange) + return 0, 0, false, newStreamErrorWithResponse(fmt.Errorf("range request on empty file %s/%s", bucket, object)) + } + + // Validate range + if startOffset < 0 || startOffset >= totalSize { + w.Header().Set("Content-Range", fmt.Sprintf("bytes */%d", totalSize)) + s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRange) + return 0, 0, false, newStreamErrorWithResponse(fmt.Errorf("invalid range start: %d >= %d, range: %s", startOffset, totalSize, rangeHeader)) + } + + if endOffset >= totalSize { + endOffset = totalSize - 1 + } + + if endOffset < startOffset { + w.Header().Set("Content-Range", fmt.Sprintf("bytes */%d", totalSize)) + s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRange) + return 0, 0, false, newStreamErrorWithResponse(fmt.Errorf("invalid range: end before start")) + } + } + + return startOffset, endOffset - startOffset + 1, true, nil +} + // StreamError is returned when streaming functions encounter errors. // It tracks whether an HTTP response has already been written to prevent // double WriteHeader calls that would create malformed S3 error responses. @@ -818,82 +907,9 @@ func (s3a *S3ApiServer) streamFromVolumeServers(w http.ResponseWriter, r *http.R // Parse Range header if present tRangeParse := time.Now() - var offset int64 = 0 - var size int64 = totalSize - rangeHeader := r.Header.Get("Range") - isRangeRequest := false - - if rangeHeader != "" && strings.HasPrefix(rangeHeader, "bytes=") { - rangeSpec := rangeHeader[6:] - parts := strings.Split(rangeSpec, "-") - if len(parts) == 2 { - var startOffset, endOffset int64 - - // Handle different Range formats: - // 1. "bytes=0-499" - first 500 bytes (parts[0]="0", parts[1]="499") - // 2. "bytes=500-" - from byte 500 to end (parts[0]="500", parts[1]="") - // 3. "bytes=-500" - last 500 bytes (parts[0]="", parts[1]="500") - - if parts[0] == "" && parts[1] != "" { - // Suffix range: bytes=-N (last N bytes) - if suffixLen, err := strconv.ParseInt(parts[1], 10, 64); err == nil { - // RFC 7233: suffix range on empty object or zero-length suffix is unsatisfiable - if totalSize == 0 || suffixLen <= 0 { - w.Header().Set("Content-Range", fmt.Sprintf("bytes */%d", totalSize)) - s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRange) - return newStreamErrorWithResponse(fmt.Errorf("invalid suffix range for empty object")) - } - if suffixLen > totalSize { - suffixLen = totalSize - } - startOffset = totalSize - suffixLen - endOffset = totalSize - 1 - } else { - // Set header BEFORE WriteHeader - w.Header().Set("Content-Range", fmt.Sprintf("bytes */%d", totalSize)) - s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRange) - return newStreamErrorWithResponse(fmt.Errorf("invalid suffix range")) - } - } else { - // Regular range or open-ended range - startOffset = 0 - endOffset = totalSize - 1 - - if parts[0] != "" { - if parsed, err := strconv.ParseInt(parts[0], 10, 64); err == nil { - startOffset = parsed - } - } - if parts[1] != "" { - if parsed, err := strconv.ParseInt(parts[1], 10, 64); err == nil { - endOffset = parsed - } - } - - // Validate range - if startOffset < 0 || startOffset >= totalSize { - // Set header BEFORE WriteHeader - w.Header().Set("Content-Range", fmt.Sprintf("bytes */%d", totalSize)) - s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRange) - return newStreamErrorWithResponse(fmt.Errorf("invalid range start for %s/%s: %d >= %d, range: %s", bucket, object, startOffset, totalSize, rangeHeader)) - } - - if endOffset >= totalSize { - endOffset = totalSize - 1 - } - - if endOffset < startOffset { - // Set header BEFORE WriteHeader - w.Header().Set("Content-Range", fmt.Sprintf("bytes */%d", totalSize)) - s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRange) - return newStreamErrorWithResponse(fmt.Errorf("invalid range: end before start")) - } - } - - offset = startOffset - size = endOffset - startOffset + 1 - isRangeRequest = true - } + offset, size, isRangeRequest, rangeErr := s3a.parseAndValidateRange(w, r, entry, totalSize, bucket, object) + if rangeErr != nil { + return rangeErr } rangeParseTime = time.Since(tRangeParse) @@ -1104,78 +1120,12 @@ func (s3a *S3ApiServer) streamFromVolumeServersWithSSE(w http.ResponseWriter, r // Parse Range header BEFORE key validation totalSize := int64(filer.FileSize(entry)) tRangeParse := time.Now() - var offset int64 = 0 - var size int64 = totalSize - rangeHeader := r.Header.Get("Range") - isRangeRequest := false - - if rangeHeader != "" && strings.HasPrefix(rangeHeader, "bytes=") { - rangeSpec := rangeHeader[6:] - parts := strings.Split(rangeSpec, "-") - if len(parts) == 2 { - var startOffset, endOffset int64 - - if parts[0] == "" && parts[1] != "" { - // Suffix range: bytes=-N (last N bytes) - if suffixLen, err := strconv.ParseInt(parts[1], 10, 64); err == nil { - // RFC 7233: suffix range on empty object or zero-length suffix is unsatisfiable - if totalSize == 0 || suffixLen <= 0 { - w.Header().Set("Content-Range", fmt.Sprintf("bytes */%d", totalSize)) - s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRange) - return newStreamErrorWithResponse(fmt.Errorf("invalid suffix range for empty object")) - } - if suffixLen > totalSize { - suffixLen = totalSize - } - startOffset = totalSize - suffixLen - endOffset = totalSize - 1 - } else { - // Set header BEFORE WriteHeader - w.Header().Set("Content-Range", fmt.Sprintf("bytes */%d", totalSize)) - s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRange) - return newStreamErrorWithResponse(fmt.Errorf("invalid suffix range")) - } - } else { - // Regular range or open-ended range - startOffset = 0 - endOffset = totalSize - 1 - - if parts[0] != "" { - if parsed, err := strconv.ParseInt(parts[0], 10, 64); err == nil { - startOffset = parsed - } - } - if parts[1] != "" { - if parsed, err := strconv.ParseInt(parts[1], 10, 64); err == nil { - endOffset = parsed - } - } - - // Validate range - if startOffset < 0 || startOffset >= totalSize { - // Set header BEFORE WriteHeader - w.Header().Set("Content-Range", fmt.Sprintf("bytes */%d", totalSize)) - s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRange) - return newStreamErrorWithResponse(fmt.Errorf("invalid range start for %s/%s: %d >= %d, range: %s", bucket, object, startOffset, totalSize, rangeHeader)) - } - - if endOffset >= totalSize { - endOffset = totalSize - 1 - } - - if endOffset < startOffset { - // Set header BEFORE WriteHeader - w.Header().Set("Content-Range", fmt.Sprintf("bytes */%d", totalSize)) - s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRange) - return newStreamErrorWithResponse(fmt.Errorf("invalid range: end before start")) - } - } - - offset = startOffset - size = endOffset - startOffset + 1 - isRangeRequest = true - glog.V(2).Infof("streamFromVolumeServersWithSSE: Range request bytes %d-%d/%d (size=%d)", startOffset, endOffset, totalSize, size) - } + offset, size, isRangeRequest, rangeErr := s3a.parseAndValidateRange(w, r, entry, totalSize, bucket, object) + if rangeErr != nil { + return rangeErr + } + if isRangeRequest { + glog.V(2).Infof("streamFromVolumeServersWithSSE: Range request bytes %d-%d/%d (size=%d)", offset, offset+size-1, totalSize, size) } rangeParseTime = time.Since(tRangeParse)