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

jsonrpc 2.0 batch request limits required #649

Closed
4 tasks
zemyblue opened this issue Jul 3, 2023 · 2 comments · Fixed by #672
Closed
4 tasks

jsonrpc 2.0 batch request limits required #649

zemyblue opened this issue Jul 3, 2023 · 2 comments · Fixed by #672
Assignees
Labels
C: proposal Classification: Proposal for specification, algorithm, architecture, or communication D: good first issue Decision: Good for newcomers

Comments

@zemyblue
Copy link
Member

zemyblue commented Jul 3, 2023

Summary

jsonrpc 2.0 batch request limits required

Problem Definition

DDoS attach is possible by requesting multiple queries in one request through jsonrpc 2.0 batch request method.
So I think it's nice to control the jsonrpc 2.0 batch request limitation.

Proposal

JSONRPC2.0 only be handled in Ostracon's RPS server using 26657 port.
And batch requests are performed in the following codes.

for _, request := range requests {
request := request
// A Notification is a Request object without an "id" member.
// The Server MUST NOT reply to a Notification, including those that are within a batch request.
if request.ID == nil {
logger.Debug(
"HTTPJSONRPC received a notification, skipping... (please send a non-empty ID if you want to call a method)",
"req", request,
)
continue
}
if len(r.URL.Path) > 1 {
responses = append(
responses,
types.RPCInvalidRequestError(request.ID, fmt.Errorf("path %s is invalid", r.URL.Path)),
)
continue
}
rpcFunc, ok := funcMap[request.Method]
if !ok || rpcFunc.ws {
responses = append(responses, types.RPCMethodNotFoundError(request.ID))
continue
}
ctx := &types.Context{JSONReq: &request, HTTPReq: r}
args := []reflect.Value{reflect.ValueOf(ctx)}
if len(request.Params) > 0 {
fnArgs, err := jsonParamsToArgs(rpcFunc, request.Params)
if err != nil {
responses = append(
responses,
types.RPCInvalidParamsError(request.ID, fmt.Errorf("error converting json params to arguments: %w", err)),
)
continue
}
args = append(args, fnArgs...)
}
returns := rpcFunc.f.Call(args)
result, err := unreflectResult(returns)
if err != nil {
responses = append(responses, types.RPCInternalError(request.ID, err))
continue
}
responses = append(responses, types.NewRPCSuccessResponse(request.ID, result))
}

So we can control the requests size if we add a limitation feature in Octracon and config.toml for example the max_request_batch_request.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@zemyblue zemyblue added D: good first issue Decision: Good for newcomers C: proposal Classification: Proposal for specification, algorithm, architecture, or communication labels Jul 3, 2023
@170210 170210 self-assigned this Jul 11, 2023
@zemyblue
Copy link
Member Author

How about setting the default max_request_batch_request as 10?

@ulbqb
Copy link
Member

ulbqb commented Jul 20, 2023

MaxBodyBytes int64 `mapstructure:"max_body_bytes"`

I think we should check if using MaxBodyBytes is enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: proposal Classification: Proposal for specification, algorithm, architecture, or communication D: good first issue Decision: Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants