Skip to content

In TLS build of library, support both TLS and non-TLS connections. #389

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

Merged
merged 1 commit into from
Feb 17, 2023

Conversation

leytou
Copy link

@leytou leytou commented Feb 15, 2023

The choice is made at runtime, if a TLS URI (https:// or wss://) is given, the TLS client will be used, otherwise the non-TLS client will be used.

Additionally, a new constructor is introduced allowing the URI to be passed at construction, which allows the above selection to occur, otherwise only the default for the library (TLS or non-TLS) will be used (preserving the original behavior).

@jmigual
Copy link
Collaborator

jmigual commented Feb 15, 2023

Amazing work! I'm impressed.

Apparently the build is failing because you are using a c++14 feature (overload 7 in https://en.cppreference.com/w/cpp/algorithm/equal) and this project is at c++11 for compatibility. Could you rewrite iequals using only c++11 feature. I would raise the minimum if you were using something quite critical but this is quite simple.

Also on Windows I'm getting a number of sections exceeded error (C1128) which I think is caused by the template implementation in the cpp file. (The TLS build is not in the CI). It's the first time I see C1128 so I'm not sure how to fix it.

@leytou leytou force-pushed the feature/tls_support_non_tls branch from 8479d94 to 6ccd4c7 Compare February 16, 2023 02:07
The choice is made at runtime, if a TLS URI (https:// or wss://) is
given, the TLS client will be used, otherwise the non-TLS client will be
used.

Additionally, a new constructor is introduced allowing the URI to be
passed at construction, which allows the above selection to occur,
otherwise only the default for the library (TLS or non-TLS) will be used
(preserving the original behavior).
@leytou leytou force-pushed the feature/tls_support_non_tls branch from 6ccd4c7 to 1b7afe6 Compare February 16, 2023 02:09
@jmigual jmigual merged commit ddd0e5b into socketio:2.x Feb 17, 2023
@jmigual
Copy link
Collaborator

jmigual commented Feb 17, 2023

Thank you!

@leytou leytou deleted the feature/tls_support_non_tls branch March 24, 2023 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants