daurnimator / lua-http

HTTP Library for Lua. Supports HTTP(S) 1.0, 1.1 and 2.0; client and server.
https://daurnimator.github.io/lua-http/
MIT License
807 stars 82 forks source link

Invalid or incomplete multibyte or wide character #225

Open les-citrons opened 3 months ago

les-citrons commented 3 months ago

if you connect to a lua-http server and send it unix newlines (which I'm aware is invalid), it will throw with a somewhat cryptic error, rather than returning an error value or tolerating it.

reproduction:

local http_server = require "http.server"

local server = assert(http_server.listen {
    host = "127.0.0.1",
    port = 8080,
    onstream = function(server, stream)
        local headers = stream:get_headers()
        if headers then
            stream:write_body_from_string("hello, world!")
        end
    end,
})
assert(server:loop())
printf 'GET / HTTP/1.1\n\n' | nc localhost 8080
lua: repro.lua:13: /usr/local/share/lua/5.4/http/server.lua:182: get_next_incoming_stream: fill: Invalid or incomplete multibyte or wide character
stack traceback:
    [C]: in function 'assert'
    repro.lua:13: in main chunk
    [C]: in ?
daurnimator commented 3 months ago

Invalid or incomplete multibyte or wide character

This is your operating system's standard error message when a function returns EILSEQ. Probably from https://github.com/daurnimator/lua-http/blob/ddab2835c583d45dec62680ca8d3cbde55e0bae6/http/h1_connection.lua#L172

lua-http generally doesn't return any error messages for such calls, only errno values (see https://en.wikipedia.org/wiki/Errno.h, EILSEQ seemed the most appropriate to me for the scenario of expecting one byte sequence but receiving another.

les-citrons commented 3 months ago

sure, I suppose \r\n is a "multibyte sequence" strictly speaking, but the error made me thought I was sending invalid UTF-8 somehow or something. maybe EPROTO would be more appropriate. but I don't feel particularly strongly about that. this isn't the actual bug.

it does return an errno as expected. but then somewhere along the line after I return from onstream, it throws an error.

onstream = function(server, stream)
    local headers, err = stream:get_headers()
    if headers then
        stream:write_body_from_string("hello, world!")
    else
        print("oh no, an error occurred. allow me to handle it properly.")
        print("I surely don't need to crash.")
    end
end
oh no, an error occurred. allow me to handle it properly.
I surely don't need to crash.
lua: repro.lua:16: /usr/local/share/lua/5.4/http/server.lua:182: get_next_incoming_stream: fill: Invalid or incomplete multibyte or wide character
stack traceback:
    [C]: in function 'assert'
    repro.lua:16: in main chunk
    [C]: in ?
daurnimator commented 3 months ago

but then somewhere along the line after I return from onstream, it throws an error.

Add an onerror: https://daurnimator.github.io/lua-http/0.4/#http.server:onerror

@@ -1,14 +1,17 @@
 local http_server = require "http.server"

 local server = assert(http_server.listen {
        host = "127.0.0.1",
        port = 8080,
        onstream = function(server, stream)
                local headers = stream:get_headers()
                if headers then
                        stream:write_body_from_string("hello, world!")
                end
        end,
+       onerror = function(server, context, op, err, errno)
+               io.stderr:write(string.format("ERROR! [%s:%d]: %s\n", op, errno, err))
+       end,
 })
 assert(server:loop())
les-citrons commented 3 months ago

while I appreciate that this does technically solve the problem, I intentionally do not use this feature because I think it is an antipattern. there's a difference between illegal characters in an HTTP request and dividing by zero. it's like on error resume next or setting a handler for SIGSEGV, though admittedly a bit less problematic. I want the program to stop when it faults, but not when it is in a situation that is expected to arise in the use of the program.

I was under the impression that I could handle error conditions in this library by checking the return values and that it would not raise errors in the case of e.g. invalid input.

daurnimator commented 3 months ago

That's what the context argument in onerror is for: in the case of this error, it's a connection level error, so the context is the connection object. You get to decide what you consider fatal vs ignorable. Anything that is fatal to the whole sever is returned as an error from :loop()

If you look at the examples you'll see that onerror often just logs the error and continues: https://github.com/daurnimator/lua-http/blob/ddab2835c583d45dec62680ca8d3cbde55e0bae6/examples/server_hello.lua#L46-L51)

les-citrons commented 3 months ago

ok, I see. so, how do I distinguish I/O errors from normal lua errors with the context argument?

edit:

If you look at the examples you'll see that onerror often just logs the error and continues:

this is what I'd like to avoid doing.