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

server: WriteRPCResponseArrayHTTP is untested and could be folded into a variadic variant of WriteRPCResponseHTTP #5135

Closed
odeke-em opened this issue Jul 18, 2020 · 0 comments · Fixed by #5141
Assignees

Comments

@odeke-em
Copy link
Contributor

odeke-em commented Jul 18, 2020

Am coming here via an audit of cosmos-sdk. The code for WriteRPCResponseArrayHTTP seems untested but also adds surface area, and duplicates a bunch of code unnecessarily, if kept perhaps could use an improved name like WriteRPCResponsesHTTP, as array is fallacious here (sorry for the nit picking)

func WriteRPCResponseHTTP(w http.ResponseWriter, res types.RPCResponse) {
jsonBytes, err := json.MarshalIndent(res, "", " ")
if err != nil {
panic(err)
}
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(200)
if _, err := w.Write(jsonBytes); err != nil {
panic(err)
}
}

func WriteRPCResponseArrayHTTP(w http.ResponseWriter, res []types.RPCResponse) {
if len(res) == 1 {
WriteRPCResponseHTTP(w, res[0])
} else {
jsonBytes, err := json.MarshalIndent(res, "", " ")
if err != nil {
panic(err)
}
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(200)
if _, err := w.Write(jsonBytes); err != nil {
panic(err)
}
}
}

but could really be folded into a variadic version of WriteRPCResponseHTTP whereby if len(res) == 1, proceed normal, lest JSON indent.

func WriteRPCResponseHTTP(w http.ResponseWriter, res ...types.RPCResponse) {
        var toMarshal interface{} = res
	if len(res) == 1 {
	        toMarshal = res[0]
        }
	jsonBytes, err := json.MarshalIndent(toMarshal, "", "  ")
	if err != nil {
		http.Error(w, err.Error(), http.StatusBadRequest)
                return
	}
	w.Header().Set("Content-Type", "application/json")
	if _, err := w.Write(jsonBytes); err != nil {
		panic(err)
	}
}

If the serve returns without error, a 200 is implicit.

@odeke-em odeke-em changed the title server: WriteRPCResponseArrayHTTP is unnecessaryuntested and could be folded into a variadic variant of WriteRPCResponseHTTP server: WriteRPCResponseArrayHTTP is untested and could be folded into a variadic variant of WriteRPCResponseHTTP Jul 18, 2020
@melekes melekes self-assigned this Jul 21, 2020
@mergify mergify bot closed this as completed in #5141 Jul 21, 2020
mergify bot pushed a commit that referenced this issue Jul 21, 2020
…#5141)

...rrayHTTP 

Closes #5135

Also, wrote a test for WriteRPCResponseHTTPError and used it with correct status codes according to https://www.jsonrpc.org/historical/json-rpc-over-http.html#response-codes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants