CrowCpp / Crow

A Fast and Easy to use microframework for the web.
https://crowcpp.org
Other
3.3k stars 364 forks source link

Missing request context when handle websocket upgrade #631

Open 1073X opened 1 year ago

1073X commented 1 year ago

In my usage, I need to retrieve loggedin user from session when websocket connecting. However the req param passed into onaccept callback has no session context. I had tried making a local modification according to how a request was manipulated in general case, it seems working as my expectation. I'm not professional on http stuffs, not sure if the missed contexts in websocket request is desired or not. Just FYI, your call to decide if this is a bug need to be fixed.

http_connection.h:134

            if (req_.check_version(1, 1)) // HTTP/1.1
            {
                if (!req_.headers.count("host"))
                {
                    is_invalid_request = true;
                    res = response(400);
                }
                else if (req_.upgrade)
                {
                    // h2 or h2c headers
                    if (req_.get_header_value("upgrade").substr(0, 2) == "h2")
                    {
                        // TODO(ipkn): HTTP/2
                        // currently, ignore upgrade header
                    }
                    else
                    {

                /*** copied from below ***/
                ctx_ = detail::context<Middlewares...>();
                req_.middleware_context = static_cast<void*>(&ctx_);
                req_.middleware_container = static_cast<void*>(middlewares_);
                req_.io_service = &adaptor_.get_io_service();

                detail::middleware_call_helper<detail::middleware_call_criteria_only_global,
                                               0, decltype(ctx_), decltype(*middlewares_)>({}, *middlewares_, req_, res, ctx_);

                        close_connection_ = true;
                        handler_->handle_upgrade(req_, res, std::move(adaptor_));
                        return;
                    }
                }
            }

            CROW_LOG_INFO << "Request: " << utility::lexical_cast<std::string>(adaptor_.remote_endpoint()) << " " << this << " HTTP/" << (char)(req_.http_ver_major + '0') << "." << (char)(req_.http_ver_minor + '0') << ' ' << method_name(req_.method) << " " << req_.url;

            need_to_call_after_handlers_ = false;
            if (!is_invalid_request)
            {
                res.complete_request_handler_ = [] {};
                res.is_alive_helper_ = [this]() -> bool {
                    return adaptor_.is_open();
                };

                ctx_ = detail::context<Middlewares...>();
                req_.middleware_context = static_cast<void*>(&ctx_);
                req_.middleware_container = static_cast<void*>(middlewares_);
                req_.io_service = &adaptor_.get_io_service();

                detail::middleware_call_helper<detail::middleware_call_criteria_only_global,
                                               0, decltype(ctx_), decltype(*middlewares_)>({}, *middlewares_, req_, res, ctx_);
The-EDev commented 1 year ago

This could be an oversight since the session middleware didn't exactly take websockets into account as far as I'm aware.