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

catchall route fails with ssl with error : Error: socket hang up #833

Open
whyjp opened this issue Jun 10, 2024 · 12 comments
Open

catchall route fails with ssl with error : Error: socket hang up #833

whyjp opened this issue Jun 10, 2024 · 12 comments
Labels
bug Something isn't working

Comments

@whyjp
Copy link

whyjp commented Jun 10, 2024

when i not include ssl cert.
i have do correctly resposnse with catch_all route.
in dynamic route on codes

but i put ssl and call with https i has not response to client
error : Error: socket hang up
and on web brouser : ERR_RESPONSE_HEADERS_TRUNCATED

when i debug codes

in async_write rise error with error codes message worng filehandle

@gittiver
Copy link
Member

If there is no SSL cert, how should a request or response be encrypted? therefore there can't be any answer on catch_all.

@whyjp
Copy link
Author

whyjp commented Jun 10, 2024

@gittiver

my explanation was wrong

  1. I tried to implement a dynamic variable path such as path/value1/path2/value2 that was not predefined through catch_all.
    Even if this was possible, until SSL was used...

When SSL is not used (no certificate is included)
It worked well this time, as intended.

However, after inserting the certificate to use SSL,
All general paths operate normally, but
through catch_all
The code that tried to respond normally by changing the 400 response code to 200 no longer worked properly.

In async_write, an error value of false appeared in ec.
At this time, the error said that the wrong file handle was accessed.

@gittiver
Copy link
Member

well, with ssl there is an error on read, as there is no cert, therefore no handshake, therefore no decryption key?
So closing socket is Ok?

@gittiver gittiver reopened this Jun 10, 2024
@gittiver gittiver added the question Issue can be closed by providing information label Jun 10, 2024
@whyjp
Copy link
Author

whyjp commented Jun 10, 2024

No i put the cert by self signed
so it works good regular paths. like loaclhost:10000/path1
but does not works in catch_all routes with ssl

@gittiver
Copy link
Member

so you have an ssl cert but it is self signed and only the catch_all route is not working, all others are working? correct?

@gittiver gittiver added bug Something isn't working discussion The viability / implementation of the issue is up for debate labels Jun 10, 2024
@whyjp
Copy link
Author

whyjp commented Jun 10, 2024

That's right, all general paths are working well.

When implementing catch_all to redirect to the correct path
Shouldn't we just change the code/body values ​​of response obj and then call res.end()?

Current code is
came in as catch_all
(with crow's 400 res code)
If the path is as intended, store and process the value.
res.code = 200 | res.body = "res text"
re.end()
I finished the code and felt that it worked well when not using SSL.

@whyjp
Copy link
Author

whyjp commented Jun 10, 2024

compile time checkable variable handler
is very useful but

I am not currently implementing the webserver directly, but I was trying to implement a more dynamic variable path.
catch_all was a valuable feature not found in other web frameworks.

However, after adding SSL, it did not work as desired.
Looking at the debug log, an error occurred in each read/write.

@gittiver gittiver removed question Issue can be closed by providing information discussion The viability / implementation of the issue is up for debate labels Jun 10, 2024
@gittiver gittiver changed the title Can not response catchall route result with ssl catchall route fails with ssl with error : Error: socket hang up Jun 10, 2024
@gittiver
Copy link
Member

sounds like a bug. rewrote the title too.

@whyjp
Copy link
Author

whyjp commented Jun 10, 2024

i try adding catchall handler
like

auto catchall = [](const crow::request& req, crow::response& res) {
        if (req.method == crow::HTTPMethod::GET) {
            std::ostringstream os;
            os << "req.url: " << req.url << "\n\n";
            os << "Params: " << req.url_params << "\n\n";
            printf("%s", os.str().c_str());
            res = crow::response(200, "application/json", R"({"res":"res force OK"})");
            return;
  }
};

with ssl cert
it cant response

thx your answer
i trying too find a bug codes in repo

@whyjp
Copy link
Author

whyjp commented Jun 12, 2024

i found key points this bugs..

when use ssl cert.

self->adaptor_.close();
in do_read()

before write ends

@whyjp
Copy link
Author

whyjp commented Jun 12, 2024

in this codes

void handle_url()
        {
            routing_handle_result_ = handler_->handle_initial(req_, res);
            // if no route is found for the request method, return the response without parsing or processing anything further.
            if (!routing_handle_result_->rule_index)
            {
                parser_.done();
                need_to_call_after_handlers_ = true;
                complete_request();
            }
        }

handle_initial
If catchall_route is encountered internally, some additional processing may be required.
Even if rule_index is 0, you should not close the socket using the code below.
Especially in the case of https

However, in the case of ssl + catchall, entering the if condition of the above code
The socket is closed before the write is completed.
A bug occurred only when using SSL.

Simply put
Only when catchall is inside get_error
found.rule_index = -1;
I confirmed that the problem was solved by temporarily adding the code, but

A more obvious code modification is

I think it would be better to have someone else who is already familiar with Crow analyze and solve the problem.
I will not reflect the code

Judging from this, it is not an error that occurs only in SSL.
Common errors in catchall case or
If maintaining the connection via ssl is not necessary, the bug will not appear in http.

@whyjp
Copy link
Author

whyjp commented Jun 13, 2024

Because the actual cause of the bug itself is
After reading ssl + request
This is because the socket is closed because it is not handled statically.
Normally, if you want to respond even with 400, you should not close the socket like this.

This bug is not limited to catch_all after all
Confirmed with ssl and handler case

I hope you change the title
@gittiver

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants