Memory leak when using render.Download

[alexei38]

Description

Memory leak when using render.Download

Steps to Reproduce the Problem

1. Create simple app

package main

import (
	"net/http"
	"os"

	"github.com/gobuffalo/buffalo"
	"github.com/gobuffalo/buffalo/render"
)

const testFilePath = "/tmp/testLargeFile"

func main() {
	app := buffalo.New(buffalo.Options{})
	app.GET("/", downloadHandler)
	app.Serve()
}

func downloadHandler(c buffalo.Context) error {
	f, _ := os.Open(testFilePath)
	return c.Render(http.StatusOK, render.Download(c, f.Name(), f))
}

2. run app

go run main.go

3. create large file

dd if=/dev/zero of=/tmp/testLargeFile bs=1M count=20000

4. download file

curl http://localhost:3000

Waiting for the OOM killer to come

Expected Behavior

The correct work is in http

func downloadHandler(c buffalo.Context) error {
	http.ServeFile(c.Response(), c.Request(), testFilePath)
	return nil
}

Actual Behavior

memory leak when working with large files

Info

-> Go: Checking installation
✓ The `go` executable was found on your system at: /home/amargasov/go/go1.17.8/bin/go

-> Go: Checking minimum version requirements
✓ Your version of Go, 1.17.5, meets the minimum requirements.

-> Go: Checking Package Management
✓ You are using Go Modules (`go`) for package management.

-> Go: Checking PATH
✓ Your PATH contains /home/amargasov/go/bin.

-> Node: Checking installation
✘ The `node` executable could not be found on your system.
For help setting up your Node environment please follow the instructions for you platform at:

https://nodejs.org/en/download/

-> NPM: Checking installation
✘ The `npm` executable could not be found on your system.
For help setting up your NPM environment please follow the instructions for you platform at:

https://docs.npmjs.com/getting-started/configuring-your-local-environment

-> Yarn: Checking installation
✘ The `yarnpkg` executable could not be found on your system.
For help setting up your Yarn environment please follow the instructions for you platform at:

https://yarnpkg.com/en/docs/install

-> PostgreSQL: Checking installation
✘ The `postgres` executable could not be found on your system.
For help setting up your Postgres environment please follow the instructions for you platform at:

https://www.postgresql.org/download/

-> MySQL: Checking installation
✘ The `mysql` executable could not be found on your system.
For help setting up your MySQL environment please follow the instructions for you platform at:

https://www.mysql.com/downloads/

-> SQLite3: Checking installation
✓ The `sqlite3` executable was found on your system at: /usr/bin/sqlite3

-> SQLite3: Checking minimum version requirements
✓ Your version of SQLite3, 3.31.1, meets the minimum requirements.

-> Cockroach: Checking installation
✘ The `cockroach` executable could not be found on your system.
For help setting up your Cockroach environment please follow the instructions for you platform at:

https://www.cockroachlabs.com/docs/stable/

-> Buffalo (CLI): Checking installation
✓ The `buffalo` executable was found on your system at: /usr/local/bin/buffalo

-> Buffalo (CLI): Checking minimum version requirements
✓ Your version of Buffalo (CLI), v0.18.2, meets the minimum requirements.

-> Buffalo: Application Details
Warning: It seems like it is not a buffalo app. (.buffalo.dev.yml not found)

[sio4]

Hi @alexei38,

Could you please explain the use case for this example? Buffalo App has dedicated function App.ServeFiles() to serve static files in a given http.FileSystem so you can use that function to serve large files. If the goal is serving large files in a directory, you can modify your app as:

func main() {
	app := buffalo.New(buffalo.Options{})
	app.ServeFiles("/", http.Dir("/tmp"))
	app.Serve()
}

Also, as you demonstrated, you can also use the standard method with function http.ServeFile() directly for specific file. (but it is not in a scope of buffalo but your application)

For the render.Download() function, it gets the name of file and io.Reader as arguments without size checking within it, so ensuring the size to prevent OOM (not memory leak) could be a responsibility of users.

Even though this function is not perfect for serving a large static file, it is still useful to serve dynamic content as a file e.g. downloading dynamically created .xls from a database table or similar purpose.

For more discussion, providing a detailed use case could be very helpful.

[alexei38]

These are static large files on the disk
Before sending there are permission checks individually for each file and other checks.
Can't use app.ServeFiles (

[github-actions]

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

[github-actions]

This issue was closed because it has been stalled for 5 days with no activity.