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

Don't serialize server errors to client. #1126

Merged
merged 1 commit into from
Nov 24, 2023

Conversation

fredrikekre
Copy link
Member

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.)

Concrete example with a server that itself sends a request to another upstream server that requires authentication:

using HTTP

function handle_request(http::HTTP.Stream)
    target = http.message.target
    if target == "/upstream"
        while !eof(http)
            @show readavailable(http)
        end
        HTTP.startwrite(http)
        error()
    else
        # Fetch something from another server and send back to client. This will err
        body = "hello, world"
        r = HTTP.post("http://localhost:8123/upstream", ["Authorization" => "hunter2"], body)
    end
end

server = HTTP.listen!(handle_request, "127.0.0.1", 8123)

This is the output from curl:

$ curl localhost:8123
HTTP.RequestError:
HTTP.Request:
HTTP.Messages.Request:
"""
POST /upstream HTTP/1.1
Authorization: hunter2                     # !!
Host: localhost:8123
Accept: */*
User-Agent: HTTP.jl/1.9.4
Content-Length: 12
Accept-Encoding: gzip

hello, world"""Underlying error:
EOFError: read end of file

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.)
@codecov-commenter
Copy link

codecov-commenter commented Nov 24, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e28d1b5) 82.70% compared to head (18edce3) 82.69%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1126      +/-   ##
==========================================
- Coverage   82.70%   82.69%   -0.01%     
==========================================
  Files          32       32              
  Lines        3053     3052       -1     
==========================================
- Hits         2525     2524       -1     
  Misses        528      528              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@fredrikekre fredrikekre merged commit 7bdb45c into JuliaWeb:master Nov 24, 2023
10 checks passed
@fredrikekre fredrikekre deleted the fe/server-errors branch November 24, 2023 14:26
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 this pull request may close these issues.

2 participants