search

Tectu / malloy

A cross-platform C++20 library providing embeddable server & client components for HTTP and WebSocket.
BSD 3-Clause "New" or "Revised" License
66 stars 8 forks source link

UB when using the default constructor of router and then adding routes #10

Closed 0x00002a closed 3 years ago

0x00002a commented 3 years ago

The default constructor of router does not initialise the logger field. This results in attempting to call a method through a null pointer whenever it tries to log. I'm guessing the reason this isn't caught on the tests is that msvc is the only one to reliably abort when this happens (gcc sometimes segfaults but the msvc runtime straight up says no). I haven't run the test suite on msvc yet but I will when I get a chance.

This was rather surprising behaviour, especially since the subrouter example suffers from this (based on the code anyway), I'm guessing it didn't show up since the router uses debug logging and spdlog by default uses info level, so it not printing anything was expected and non-msvc just ignored it. I'm not really sure what the best way to resolve this is, just saying as someone trying out the library, it was surprising.

Mostly a breeze to use so far though, great work- gives me django vibes :p

Tectu commented 3 years ago

Good catch. As mentioned I have only compiled this with GCC & Clang so far. I am very thankful for this kind of feedback. This certainly needs to be rectified.

Mostly a breeze to use so far though, great work- gives me django vibes :p

That is good to hear - It's exactly what I am after. A django/flask/whatever kind of experience but then native, modern C++. I was wondering whether I should add (optional) built-in support for nlohmann::json and pantor/inja. I am already using these components in an application that uses malloy. This way we could also add built-in tooling for building REST APIs etc.

Please keep the feedback & pull requests coming. It's greatly appreciated :)

Tectu commented 3 years ago

In the meantime I also removed all ciso64 keywords as MSVC does not like them out-of-the-box as I understand: dd21ec7d3e300d9b271737cf67cc90fd25ac9ce4

0x00002a commented 3 years ago

Yeah thats weird that it compiled that, it usually chokes on those. What would be nice is if the regex route supported capture groups, maybe passed in as a vector or something. Currently I'm doing my own parsing of the request's target in order to grab the bits I need from it. Could wrap the version without the extra args in a lambda before storing it or something.

Tectu commented 3 years ago

I agree, That would be a good feature and should be easy to implement. I'd recommend you to open a separate issue for this so we can get this done :)