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

SPARK-2338 Refactor: rename the confusing "Old SSL" to "Direct TLS" #875

Merged
merged 6 commits into from
Aug 24, 2024

Conversation

stokito
Copy link
Contributor

@stokito stokito commented Aug 24, 2024

The code uses a term "Old SSL" for Direct TLS connections on port 5223.
This is supported by XEP-0368 and may be not necessary for old clients.
To make the code follow same terminology as XEP I renamed the LocalPreferences.isSSL() to isDirectTls(). The property name in properties file internally remains same.

Additionally extract the similar code to configure TLS in the Login and AccountCreationWizard.

@stokito
Copy link
Contributor Author

stokito commented Aug 24, 2024

Check the comment:

 SMACK 4.1.9 does not support XEP-0368, and does not apply a port change, if the host is not changed too.
 Here, we force the host to be set (by doing a DNS lookup), and force the port to 5223 (which is the default 'old-style' SSL port).

It looks like this Direct TLS code should be moved into the Smack library. Probably to this place https://github.com/igniterealtime/Smack/blob/4aacdc5154e7eb55df175bf661abbb627f6f76a7/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XmppTcpTransportModule.java#L626

@Plyha Plyha changed the title Refactor: rename the confusing "Old SSL" to "Direct TLS" SPARK-2338 Refactor: rename the confusing "Old SSL" to "Direct TLS" Aug 24, 2024
@Plyha
Copy link
Member

Plyha commented Aug 24, 2024

As I understand it, Direct TLS is depricated, I don't think it will be added to Smack, but it's worth telling @Flowdalic about it

@Plyha Plyha merged commit 3da5d35 into igniterealtime:master Aug 24, 2024
4 checks passed
@stokito stokito deleted the direct_tls branch August 24, 2024 10:35
@stokito
Copy link
Contributor Author

stokito commented Aug 24, 2024

from the XEP:

It also provides an easy way for clients to bypass restrictive firewalls that only allow HTTPS, for servers to host multiple protocols/services on a single port, and for servers and clients to take advantage of less round trips and existing direct TLS loadbalancers.

So it's useful and basically can be even better.

Also please check the last commit 6ee0092. I extracted two methods that looks similar but maybe this should be the same method. It looks like the AccountCreationWizzard is just has an outdated version.

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.

2 participants