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

Qt5 fixes #2347

Closed
wants to merge 2 commits into from
Closed

Qt5 fixes #2347

wants to merge 2 commits into from

Conversation

m-kuhn
Copy link
Member

@m-kuhn m-kuhn commented Oct 4, 2015

Patches the new auth system to compile against Qt5 (See commit message for more information)

Fixes some additional warnings when compiling for Qt5.

@nirvn
Copy link
Contributor

nirvn commented Oct 4, 2015

@m-kuhn, @dakcarto , that fixes compilation errors, excellent.

> QSslCertificate::subjectInfo() and QSslCertificate::issuerInfo() return a
> QStringList instead of a QString in Qt5. It's pretty common for a certificate
> to contain more than entry of a specific type, but in Qt 4 the API only let
> you access the first one.

[Source](https://blogs.kde.org/2012/04/14/whats-new-qt-5-ssl)

This fix adds macros to apply the Qt4 behavior on Qt5 code. Obviously it would
be better the other way round.
@dakcarto
Copy link
Member

dakcarto commented Oct 5, 2015

@m-kuhn

These changes look fine to me. I have not tested the auth system code against Qt5 (obviously). I think there may be several other places that need some work, relative to changes through Qt 5.5:

  • enum QSsl::KeyAlgorithm - Elliptic Curve algorithms are new, though unclear to me how prevalent their use is in client PKI bundles, which is the only private key support in the auth system
  • enum QSsl::SslProtocol - Introduces changes to enums, but looks to be compatible with SSL server configuration still

+1 to merge

@m-kuhn
Copy link
Member Author

m-kuhn commented Oct 6, 2015

@dakcarto thanks for the review. I did not check the auth code either, at the moment I'm happy as long as we get the code through the compiler and linker with Qt5 to get a gradually increasing amount of testers and improved codebase.

One thing that came to my mind though, with Qt4 there was always only one subject info and issuer info. With Qt5 there could be multiple. Now we replicate the Qt4 behavior to always check the first one. But I could imagine that it would be good to use the whole list with Qt5 (they probably changed it to a list for a reason). But I am not sure if in this case it needs to match all or any and if this is even something that should be considered important.

@m-kuhn m-kuhn closed this Oct 6, 2015
@m-kuhn
Copy link
Member Author

m-kuhn commented Oct 6, 2015

Merged in dc04314

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.

None yet

3 participants