Skip to content

Commit

Permalink
Don't serialize server errors to client.
Browse files Browse the repository at this point in the history
Server side errors may contain just about anything, such as e.g.
secrets, and, therefore, it seems like a bad idea to unconditionally
send these back to the client. In general, there is nothing a client can
do about an internal server error even if the specific internal error
message is known. The server developer can already see the error in
server logs so there isn't really any loss of information.

If the current behavior is actually desired it can be achieved by an
outer `try-catch` in the handler function. (Of course, an outer
`try-catch` can also be used to make sure that a server side error never
ends up at the client, but it is better to be safe by default.)
  • Loading branch information
fredrikekre committed Nov 24, 2023
1 parent e28d1b5 commit 18edce3
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 3 deletions.
1 change: 0 additions & 1 deletion src/Servers.jl
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,6 @@ function handle_connection(f, c::Connection, listener, readtimeout, access_log)
if isopen(http) && !iswritable(http)
request.response.status = 500
startwrite(http)
write(http, sprint(showerror, e))
closewrite(http)
end
c.state = CLOSING
Expand Down
3 changes: 1 addition & 2 deletions test/client.jl
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ end
shouldfail[] = true
seekstart(req_body)
resp = HTTP.get("http://localhost:8080/retry"; body=req_body, response_stream=res_body, retry=false, status_exception=false)
@test String(take!(res_body)) == "500 unexpected error"
@test resp.status == 500
# even if StatusError, we should still get the right response body
shouldfail[] = true
seekstart(req_body)
Expand All @@ -620,7 +620,6 @@ end
catch e
@test e isa HTTP.StatusError
@test e.status == 500
@test String(take!(res_body)) == "500 unexpected error"
end
# don't throw a 500, but set status to status we don't retry by default
shouldfail[] = false
Expand Down

0 comments on commit 18edce3

Please sign in to comment.