TLS/HTTPS (semi) support

[dobby5]

See: #39 (comment)

[dobby5]

If there is a merge please change this line with your branch @getnamo

[getnamo]

I'm not a fan of overriding SSL support by dropping non-ssl as that mode is far more common in use case. What can be done is to merge this into a ssl branch until we figure out a way to swap support via url lookup (https -> ssl, http-> standard)

[getnamo]

added merge to secondary branch here: https://github.com/getnamo/socketio-client-ue4/tree/ssl

[getnamo]

open future pull requests there until we manage to support both methods and it becomes ready for master

[getnamo]

I think I might have an idea on how to get the url based switch support. Will try to find a day or two to explore it.

We need to de-couple the constructor https://github.com/getnamo/socketio-client-ue4/blob/d0cad0135c1aca1ea4d891697c21f96767741fc0/Source/SocketIOLib/Private/internal/sio_client_impl.cpp#L51 or pass in a variable in it to selectively attach https://github.com/getnamo/socketio-client-ue4/blob/d0cad0135c1aca1ea4d891697c21f96767741fc0/Source/SocketIOLib/Private/internal/sio_client_impl.cpp#L78,

chasing the chain: https://github.com/getnamo/socketio-client-ue4/blob/d0cad0135c1aca1ea4d891697c21f96767741fc0/Source/SocketIOLib/Private/sio_client.cpp#L35
then at the top we'd just need to fix https://github.com/getnamo/socketio-client-ue4/blob/d0cad0135c1aca1ea4d891697c21f96767741fc0/Source/SocketIOClient/Private/SocketIONative.cpp#L23

and adjust https://github.com/getnamo/socketio-client-ue4/blob/d0cad0135c1aca1ea4d891697c21f96767741fc0/Source/SocketIOClient/Private/SocketIONative.cpp#L71 to ensure it causes the SSL swap trigger (but only on supported platforms).

[dobby5]

I thing it is possible to including a variable at the said places, but I see the problem mainly at the place https://github.com/getnamo/socketio-client-ue4/blob/d0cad0135c1aca1ea4d891697c21f96767741fc0/Source/SocketIOLib/Private/internal/sio_client_impl.h#L38-L62, in the inclusion of the SSL or non-SSL hpp data.

[getnamo]

Let's see that's used in https://github.com/getnamo/socketio-client-ue4/blob/d0cad0135c1aca1ea4d891697c21f96767741fc0/Source/SocketIOLib/Private/internal/sio_client_impl.h#L81 which is used in https://github.com/getnamo/socketio-client-ue4/blob/d0cad0135c1aca1ea4d891697c21f96767741fc0/Source/SocketIOLib/Private/internal/sio_client_impl.cpp#L283 and https://github.com/getnamo/socketio-client-ue4/blob/d0cad0135c1aca1ea4d891697c21f96767741fc0/Source/SocketIOLib/Private/internal/sio_client_impl.cpp#L470 and https://github.com/getnamo/socketio-client-ue4/blob/d0cad0135c1aca1ea4d891697c21f96767741fc0/Source/SocketIOLib/Private/internal/sio_client_impl.cpp#L514.

We might be able to keep two type of configs and pass them in depending on connection types I think. The above is only a typedef so I do think we can keep two types

[getnamo]

Yeah we'd need a different handler for on_message in tls variant https://github.com/getnamo/socketio-client-ue4/blob/d0cad0135c1aca1ea4d891697c21f96767741fc0/Source/SocketIOLib/Private/internal/sio_client_impl.cpp#L76 with the tls config, this can be changed on construction.

High level: On connect -> if mode is mismatching url, we'd re-create the lib client with correct mode and then connect. Each component has it's own client so we should be able to interweave tls and non tls connections

[dobby5]

Good morning, if I understand it correctly, your goal is basically to make these changes, right?

socketio/socket.io-client-cpp#137

[dobby5]

I have taken the changes from joelnordell and adapted them to the new version of our socket.io-client-cpp.

I still get an error with the "class/struct" maybe you have an idea how we can fix this.

I have pushed the changes into a new branch:
https://github.com/dobby5/socketio-client-ue4/tree/tls_testing

[getnamo]

Going to work on it within #267

At a first glance, I'm not fully convinced at the templated approach joelnordell did, seems unnecessarily complicated, but if it works, it could be a quick enough patch. Will know more later today.

[getnamo]

Got it to compile and pulled into https://github.com/getnamo/socketio-client-ue4/commits/ssl-dev

Emit still works, but receiving events broke, TBC.

Update: After seeing the current changes breaking on-receive without matching binding functions, I'm back to not being convinced a templated variant is the right approach. Will push this back one week and re-factor so it's easier to reason.

[dobby5]

Hello @getnamo,

I hope that we can find a quick fix for this. I will have a look at your changes during the day. Maybe you will find another approach to solution. I am also curious how you will solve it.

[dobby5]

@getnamo Do you have any news? Good and fast work by the way with the UE5 update.

[getnamo]

Still pushing the SSL refactor for next week (it's a lot more involved than the ue5 upgrade) as I'm out of time for sio this week. Will know more then.

[dobby5]

Okay good