Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] When uploading normative routing files, the memory overhead is too large #2938

Closed
wlynxg opened this issue Sep 6, 2023 · 2 comments · Fixed by #3178
Closed

[Bug] When uploading normative routing files, the memory overhead is too large #2938

wlynxg opened this issue Sep 6, 2023 · 2 comments · Fixed by #3178
Labels
bug It is confirmed a bug, but don't worry, we'll handle it. help wanted planned This issue/proposal is planned into our next steps.

Comments

@wlynxg
Copy link
Contributor

wlynxg commented Sep 6, 2023

When standard routing is used, gf will read the entire body content first, and then do parameter binding. Therefore, when there is a file parameter in the canonical route, gf will first read the content of the file into memory. When there are multiple file upload requests at the same time, it is easy to blow up the system memory.

1. What version of Go and system type/arch are you using?

go version go1.20 linux/amd64

2. What version of GoFrame are you using?

v2.5.2

3. Can this issue be re-produced with the latest release?

yes

4. What did you do?

package main

import (
	"context"

	"github.com/gogf/gf/v2/frame/g"
	"github.com/gogf/gf/v2/net/ghttp"
)

type UploadReq struct {
	g.Meta `path:"/upload" method:"POST" tags:"Upload" mime:"multipart/form-data" summary:"上传文件"`
	File   *ghttp.UploadFile `p:"file" type:"file" dc:"选择上传文件"`
}
type UploadRes struct{}

type cUpload struct{}

func (u cUpload) Upload(ctx context.Context, req *UploadReq) (*UploadRes, error) {
	return nil, nil
}

func main() {
	s := g.Server()
	s.Group("/", func(group *ghttp.RouterGroup) {
		group.Bind(cUpload{})
	})
        s.SetClientMaxBodySize(100 * 1024 * 1024)
	s.Run()
}

5. What did you expect to see?

When there are multiple upload requests at the same time, the memory overhead will not be too large.

6. What did you see instead?

Read the file data and write the data to the disk at the same time, instead of reading all the data and then writing it to the disk.

@gqcn
Copy link
Member

gqcn commented Oct 18, 2023

@wlynxg Hi, thanks for notice. Yes, there is a risk as you described. The risk lays in the function parseBody of Request. There should be some improvement for parameters parsing from request body. For example, checking the request content type or the posted form type, it can ignore the body parsing if the content type containing multipart/ or form.
image

@gqcn gqcn added bug It is confirmed a bug, but don't worry, we'll handle it. help wanted planned This issue/proposal is planned into our next steps. labels Oct 18, 2023
@github-actions
Copy link

Hello @wlynxg. We like your proposal/feedback and would appreciate a contribution via a Pull Request by you or another community member. We thank you in advance for your contribution and are looking forward to reviewing it!
你好 @wlynxg。我们喜欢您的提案/反馈,并希望您或其他社区成员通过拉取请求做出贡献。我们提前感谢您的贡献,并期待对其进行审查。

hailaz added a commit that referenced this issue Nov 27, 2023
@hailaz hailaz mentioned this issue Nov 27, 2023
gqcn pushed a commit that referenced this issue Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug It is confirmed a bug, but don't worry, we'll handle it. help wanted planned This issue/proposal is planned into our next steps.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants