Repository: gogs/gogs
File: internal/lfsutil/storage.go (Go)

Code before:
	defer func() {
		rc.Close()

		if err != nil {
			_ = os.Remove(fpath)
		}
	}()

	err = os.MkdirAll(filepath.Dir(fpath), os.ModePerm)
	if err != nil {
		return 0, errors.Wrap(err, "create directories")
	}

	w, err := os.OpenFile(fpath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o666)
	if err != nil {
		if os.IsExist(err) {
			// The file was already written by a previous upload (possibly for a
			// different repo). Since the OID is a content hash, the existing file
			// is guaranteed to have the correct content. Drain the reader and
			// return the existing file's size.
			_, _ = io.Copy(io.Discard, rc)
			fi, statErr := os.Stat(fpath)
			if statErr != nil {
				err = errors.Wrap(statErr, "stat existing file")
				return 0, err
			}
			err = nil
			return fi.Size(), nil
		}
		return 0, errors.Wrap(err, "create file")
	}
	defer w.Close()

	hash := sha256.New()
	written, err := io.Copy(w, io.TeeReader(rc, hash))
	if err != nil {
		return 0, errors.Wrap(err, "copy file")
	}

	if computed := hex.EncodeToString(hash.Sum(nil)); computed != string(oid) {
		err = ErrOIDMismatch
		return 0, err
	}
	return written, nil
}

func (s *LocalStorage) Download(oid OID, w io.Writer) error {
	fpath := s.storagePath(oid)
	if !osutil.IsFile(fpath) {
		return ErrObjectNotExist
	}

Review comment (bug):
In the `os.IsExist(err)` branch, if `os.Stat(fpath)` fails you assign to `err` and return. Because the outer `defer` removes `fpath` on any non-nil `err`, this can end up deleting a pre-existing LFS object that this call did not create. Consider tracking whether this invocation created the file (e.g., a `created`/`cleanup` flag set only after successful create) and only removing on error when `created` is true.

Code after:
	fpath := s.storagePath(oid)
	dir := filepath.Dir(fpath)

	defer rc.Close()

	if err := os.MkdirAll(dir, os.ModePerm); err != nil {
		return 0, errors.Wrap(err, "create directories")
	}

	// If the object file already exists, skip the upload and return the
	// existing file's size.
	if fi, err := os.Stat(fpath); err == nil {
		_, _ = io.Copy(io.Discard, rc)
		return fi.Size(), nil
	}

	// Write to a temp file and verify the content hash before publishing.
	// This ensures the final path always contains a complete, hash-verified
	// file, even when concurrent uploads of the same OID race.
	if err := os.MkdirAll(s.TempDir, os.ModePerm); err != nil {
		return 0, errors.Wrap(err, "create temp directory")
	}
	tmp, err := os.CreateTemp(s.TempDir, "upload-*")
	if err != nil {
		return 0, errors.Wrap(err, "create temp file")
	}
	tmpPath := tmp.Name()
	defer os.Remove(tmpPath)

	hash := sha256.New()
	written, err := io.Copy(tmp, io.TeeReader(rc, hash))
	if closeErr := tmp.Close(); err == nil && closeErr != nil {
		err = closeErr
	}
	if err != nil {
		return 0, errors.Wrap(err, "write object file")
	}

	if computed := hex.EncodeToString(hash.Sum(nil)); computed != string(oid) {
		return 0, ErrOIDMismatch
	}

	if err := os.Rename(tmpPath, fpath); err != nil && !os.IsExist(err) {
		return 0, errors.Wrap(err, "publish object file")
	}
	return written, nil
}

func (s *LocalStorage) Download(oid OID, w io.Writer) error {
	fpath := s.storagePath(oid)
	if !osutil.IsFile(fpath) {