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

[qtnetwork] Replace SSL verification error locks #11

Open
wants to merge 1 commit into
base: mer-5.6
Choose a base branch
from

Conversation

HenkKalkwater
Copy link

This pull request should solve QTBUG-76157 by removing the mutex-based error list and instead attaching the error list to either the X509_STORE or SSL struct as ex_data.

I couldn't get the Qt unit tests working, but I have tested this on my device with a small application that I wrote for connecting with servers with known bad SSL certificates and it seems to work as before.

Additionally, my own app Sailfin, does no longer freeze when connecting over HTTPS, so it does solve the problem for me.

I only have tested this on Sailfish 4.4.0.68. I assumed keeping support for older OpenSSL versions than 1.1.0 wasn't necessary, therefore building with older versions than 1.1.0 may fail. Please let me know if that would be an issue.

Related forum topic: https://forum.sailfishos.org/t/possibility-of-backporting-qt-bug-fixes/11424

This commit should solve QTBUG-76157 by removing the mutex-based error
list and instead attaching the error list to either the X509_STORE or
SSL struct as ex_data.

This bug has been fixed in upstream Qt since 5.12, so this commit may
be ignored when upgrading to that Qt version or higher.
@HenkKalkwater
Copy link
Author

HenkKalkwater commented Aug 18, 2022

After having tested this for a few weeks, nothing seems to crash. However, I only tested it without the booster and then it works. When launching apps with the booster it stops reporting errors for some reason. This obviously shouldn't happen, so I'd recommend against merging this in right now.

I didn't give my test app the required internet permission, so all the SSL tests would fail. Now it has the internet permission and it turns out it works just fine.

@HenkKalkwater HenkKalkwater marked this pull request as draft August 18, 2022 11:43
@HenkKalkwater HenkKalkwater marked this pull request as ready for review August 18, 2022 12:09
@HenkKalkwater
Copy link
Author

For clarity, because I may have been a bit chaotic with my communication in the past, the current pull request is ready for review and there are no issues that I'm aware off.

@HenkKalkwater HenkKalkwater changed the title [qtnetwork] Remove SSL verification error locks [qtnetwork] Replace SSL verification error locks Nov 8, 2022
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.

1 participant