JuliaWeb / HTTP.jl

HTTP for Julia
https://juliaweb.github.io/HTTP.jl/stable/
Other
626 stars 177 forks source link

Don't serialize server errors to client. #1126

Closed fredrikekre closed 7 months ago

fredrikekre commented 7 months ago

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
codecov-commenter commented 7 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

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

Additional details and impacted files ```diff @@ 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 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.