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

No error will be reported for illegal JSON requests #4087

Open
lingcoder opened this issue Dec 30, 2024 · 3 comments
Open

No error will be reported for illegal JSON requests #4087

lingcoder opened this issue Dec 30, 2024 · 3 comments
Labels
bug It is confirmed a bug, but don't worry, we'll handle it. help wanted

Comments

@lingcoder
Copy link
Contributor

Go version

go version go1.23.3 windows/amd64

GoFrame version

v2.8.3

Can this bug be reproduced with the latest release?

Option Yes

What did you do?

使用默认的规范路由写法定义controller层出参入参,默认应该是application/json 请求。
前端传递了非法的JSON。gf没有任何错误提醒。

What did you see happen?

request := g.RequestFromCtx(ctx)
fmt.Println(request.GetBodyString())

d3c6b0f321c1d9bb44462dea43b55ce

What did you expect to see?

应该提示参数格式不对吧?

@lingcoder lingcoder added the bug It is confirmed a bug, but don't worry, we'll handle it. label Dec 30, 2024
@Issues-translate-bot Issues-translate-bot changed the title 非法的JSON请求不报错 No error will be reported for illegal JSON requests Dec 30, 2024
@gqcn
Copy link
Member

gqcn commented Jan 22, 2025

@lingcoder 唔,因吹斯汀,ghttp.Server在处理Body的解析的时候并没有判断提交的ContentType类型值,并且识别JSON格式的时候也不太严谨,同时忽略了解析错误:

func (r *Request) parseBody() {

这块有很大的改进空间:

  1. parseBody应当返回error,并且针对其他内部解析方法parseQuery/parseForm也应当返回error,不应当忽略,也不应当panic
  2. 在针对Body内容进行解析时:
    • 优先识别ContentType提交的类型值,如果是JSON格式并且JSON解析失败,应当返回报错。
    • 如果没有ContentType提交,那么保留现有的格式判断,并可以忽略格式解析错误。
  3. 解析错误应当SetError到当前的ghttp.Request中,以便更上层能捕获执行自定义错误处理。

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


@lingcoder Well, because of Sting, ghttp.Server did not judge the submitted ContentType type value when processing Body parsing, and was not very strict when recognizing the JSON format, and ignored parsing errors. : https://g ithub.com/gogf/gf/blob/e1fa99013a02cd76c5297a896a83a2ba74a77c2a/net/ghttp/ghttp_request_param.go#L228

There is a lot of room for improvement in this area:

  1. parseBody should return error, and other internal parsing methods parseQuery/parseForm should also return error, which should not be ignored or panic.
  2. When parsing the content of Body:
    • Prioritize the identification of the type value submitted by ContentType. If it is in JSON format and JSON parsing fails, an error should be returned.
    • If no ContentType is submitted, the existing format judgment is retained and format parsing errors can be ignored.
  3. Parsing errors should SetError to the current ghttp.Request so that the upper layer can capture and perform custom error handling.

Copy link

Hello @lingcoder. 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!
你好 @lingcoder。我们喜欢您的提案/反馈,并希望您或其他社区成员通过拉取请求做出贡献。我们提前感谢您的贡献,并期待对其进行审查。

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
Projects
None yet
Development

No branches or pull requests

3 participants