Re: [Development] [API Change] New authentication method in QNetworkAccessManager
On 09 Mar 2014, at 22:46, Richard Moore r...@kde.org wrote: IIRC SSL sockets had the same issue when SSL errors are raised. Has it been solved there? How? AFAIK, it has not been solved. The problem is the same. It has been partially solved, in recent versions you can set the socket to pause when ssl errors occur which avoids the need to use a nested event loop, see https://codereview.qt-project.org/#change,13834 We want this to be supported for authentication and also for ssl when there are no errors. It is being tracked as QTBUG-19032. Is the documentation on the signal sslErrors() still correct then: Note: You cannot use Qt::QueuedConnection when connecting to this signal, or calling QSslSocket::ignoreSslErrors() will have no effect. ? Do you think this mechanism should also be added to QWebSockets, or leave it as it is? Personally, I think that the delegate approach is a nicer and for the end-programmer a less error prone solution. It could even be combined with the pause/resume functionality: when a delegate is provided, pause the socket, asynchronously invoke the delegate and resume the socket when the delegate throws a signal. I know this would be for a next major release, but is it worth trying this out and propose it as an extension to the API? Cheers, Kurt Rich. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] [API Change] New authentication method in QNetworkAccessManager
On 10 March 2014 07:43, Kurt Pattyn pattyn.k...@gmail.com wrote: Is the documentation on the signal sslErrors() still correct then: Note: You cannot use Qt::QueuedConnection when connecting to this signal, or calling QSslSocket::ignoreSslErrors() will have no effect. ? I presume it's still correct in order to retain compatibility with Qt 4. The new pausing mechanism is opt-in: you need to call setPauseMode(QAbstractSocket::PauseOnSslErrors) in order to make the socket pause when emitting sslErrors(). Once that signal gets emitted, the socket pauses and will not proceed further unless the user then calls ignoreSslErrors() + resume() or so. To me this mechanism seems flexible enough to be coupled with anything... -- Giuseppe D'Angelo ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] [API Change] New authentication method in QNetworkAccessManager
On 10 Mar 2014, at 08:37, Giuseppe D'Angelo dange...@gmail.com wrote: On 10 March 2014 07:43, Kurt Pattyn pattyn.k...@gmail.com wrote: Is the documentation on the signal sslErrors() still correct then: Note: You cannot use Qt::QueuedConnection when connecting to this signal, or calling QSslSocket::ignoreSslErrors() will have no effect. ? I presume it's still correct in order to retain compatibility with Qt 4. The new pausing mechanism is opt-in: you need to call setPauseMode(QAbstractSocket::PauseOnSslErrors) in order to make the socket pause when emitting sslErrors(). Once that signal gets emitted, the socket pauses and will not proceed further unless the user then calls ignoreSslErrors() + resume() or so. But we can use a QueuedConnection if the PauseMode is set to PauseOnSslErrors, or can't we? To me this mechanism seems flexible enough to be coupled with anything... -- Giuseppe D'Angelo ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] [API Change] New authentication method in QNetworkAccessManager
2014-03-09 16:10 GMT+02:00 Kurt Pattyn pattyn.k...@gmail.com: Currently, QNAM stalls when authentication is required (also see: https://bugreports.qt-project.org/browse/QTBUG-16251). Also, the connection between the authenticationRequired signal and the slot must be a direct connection. This is problematic when an application wants to show a login dialogbox for instance. QAuthenticator doesn't seem to make any difference in this case. Am I missing something? Anyways, this sounds more convenient than the current behavior. I propose to change this implementation by using a delegate authenticator instead. class QAuthenticator { … protected Q_SLOTS: //this method should be called asynchronously from QNAM void authenticate(QNetworkReply *reply) { if (doAuthenticate(reply)) Q_EMIT authenticated(); else Q_EMIT authenticationFailed(); } ... protected: //return false if the request should be stopped (authentication denied) //return true if authentication should continue virtual bool doAuthenticate(QNetworkReply *reply) = 0; ... } //example use class MyAuthenticator: public QAuthenticator { bool doAuthenticate(QNetworkReply *reply) { setUser(); setPassword(); setOption(); //whatever is required return true; } } The QNetworkRequest class would be extended with a setAuthenticator() method, as follows: class QNetworkRequest { ... void setAuthenticator(QAuthenticator *authenticator); ... } Usage: MyAuthenticator *auth = new MyAuthenticator; networkRequest.setAuthenticator(auth); Another option is to add the setAuthenticator() method to the QNAM class. When this would be implemented, the authenticationRequired() signal would become obsolete. What are your opinions on this? Note: QWebSocketServer uses the same blocking signal for CORS authentication (this was merely done to be inline with the QNAM way of working). Cheers, Kurt ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] [API Change] New authentication method in QNetworkAccessManager
On Sunday, 2014-03-09, 15:10:02, Kurt Pattyn wrote: Currently, QNAM stalls when authentication is required (also see: https://bugreports.qt-project.org/browse/QTBUG-16251). Also, the connection between the authenticationRequired signal and the slot must be a direct connection. This is problematic when an application wants to show a login dialogbox for instance. I propose to change this implementation by using a delegate authenticator instead. class QAuthenticator { … protected Q_SLOTS: QAuthenticator is not a QObject, right? Cheers, Kevin -- Qt Developer Days 2014, October 6 - 8 at BCC, Berlin. Save the dates! Kevin Krammer | kevin.kram...@kdab.com | Senior Software Engineer Klarälvdalens Datakonsult AB, a KDAB Group company Tel. Sweden (HQ) +46-563-540090, USA +1-866-777-KDAB(5322) KDAB - Qt Experts - Platform-independent software solutions smime.p7s Description: S/MIME cryptographic signature ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] [API Change] New authentication method in QNetworkAccessManager
On 09 Mar 2014, at 17:16, Konstantin Ritt ritt...@gmail.com wrote: 2014-03-09 16:10 GMT+02:00 Kurt Pattyn pattyn.k...@gmail.com: Currently, QNAM stalls when authentication is required (also see: https://bugreports.qt-project.org/browse/QTBUG-16251). Also, the connection between the authenticationRequired signal and the slot must be a direct connection. This is problematic when an application wants to show a login dialogbox for instance. QAuthenticator doesn't seem to make any difference in this case. Am I missing something? Well, QAuthenticator would have to be extended with a slot and 1 or 2 signals. Anyways, this sounds more convenient than the current behavior. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] [API Change] New authentication method in QNetworkAccessManager
On Sunday 09 March 2014 15:10:02 Kurt Pattyn wrote: Currently, QNAM stalls when authentication is required (also see: https://bugreports.qt-project.org/browse/QTBUG-16251). Also, the connection between the authenticationRequired signal and the slot must be a direct connection. This is problematic when an application wants to show a login dialogbox for instance. I propose to change this implementation by using a delegate authenticator instead. ... What are your opinions on this? 1) Your suggested change would be binary incompatible. You would need to pick another class name than QAuthenticator 2) Your change does not seem to fix the problem at all. You get a blocking virtual function instead of a blocking signal. Here is my suggested API change: adding QNetworkReply::pause and QNetworkReply::resumeAuthentication, to be used as this. QObject::connect(qnam, QNetworkManager::authenticationRequired, [](QNetworkReply * reply, QAuthenticator * authenticator) { reply-pause(); // New slot: when we return this function it won't continue auto dialog = new MyPasswordDialog(); dialog-open(); QObject::connect(dialog, MyPasswordDialog::done, reply, [=]{ authenticator-setUser(dialog-user); authenticator-setPassword(dialog-password); reply-resumeAuthentication(authenticator); //new slot; }); }); Alternative: adding those function to the QAuthenticator itself. -- Olivier Woboq - Qt services and support - http://woboq.com - http://code.woboq.org ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] [API Change] New authentication method in QNetworkAccessManager
On 9 March 2014 15:10, Kurt Pattyn pattyn.k...@gmail.com wrote: Also, the connection between the authenticationRequired signal and the slot must be a direct connection. IIRC SSL sockets had the same issue when SSL errors are raised. Has it been solved there? How? -- Giuseppe D'Angelo ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] [API Change] New authentication method in QNetworkAccessManager
On 09 Mar 2014, at 19:32, Olivier Goffart oliv...@woboq.com wrote: On Sunday 09 March 2014 15:10:02 Kurt Pattyn wrote: Currently, QNAM stalls when authentication is required (also see: https://bugreports.qt-project.org/browse/QTBUG-16251). Also, the connection between the authenticationRequired signal and the slot must be a direct connection. This is problematic when an application wants to show a login dialogbox for instance. I propose to change this implementation by using a delegate authenticator instead. ... What are your opinions on this? 1) Your suggested change would be binary incompatible. You would need to pick another class name than QAuthenticator 2) Your change does not seem to fix the problem at all. You get a blocking virtual function instead of a blocking signal. Here is my suggested API change: adding QNetworkReply::pause and QNetworkReply::resumeAuthentication, to be used as this. QObject::connect(qnam, QNetworkManager::authenticationRequired, [](QNetworkReply * reply, QAuthenticator * authenticator) { reply-pause(); // New slot: when we return this function it won't continue auto dialog = new MyPasswordDialog(); dialog-open(); QObject::connect(dialog, MyPasswordDialog::done, reply, [=]{ authenticator-setUser(dialog-user); authenticator-setPassword(dialog-password); reply-resumeAuthentication(authenticator); //new slot; }); }); Alternative: adding those function to the QAuthenticator itself. I think it does solve the problem if QNAM would be changed like this: if (networkRequest-hasAuthenticator()) connect(networkRequest-authenticator(), QAuthenticator::authenticated, this, QNAM::goToNextStep); ... if (authenticationNeeded) if (networkRequest-hasAuthenticator()) QMetaMethod::invoke(“authenticate , networkRequest, ..., QueuedConnection); else //authentication failed else goToNextStep(); ... Q_SLOTS: void goToNextStep() { ... } The handling of incoming data would basically be split in two parts. When authentication is needed, a queued metacall is invoked which makes it asynchronous. -- Olivier Woboq - Qt services and support - http://woboq.com - http://code.woboq.org ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] [API Change] New authentication method in QNetworkAccessManager
On 09 Mar 2014, at 21:02, Giuseppe D'Angelo dange...@gmail.com wrote: On 9 March 2014 15:10, Kurt Pattyn pattyn.k...@gmail.com wrote: Also, the connection between the authenticationRequired signal and the slot must be a direct connection. IIRC SSL sockets had the same issue when SSL errors are raised. Has it been solved there? How? AFAIK, it has not been solved. The problem is the same. -- Giuseppe D'Angelo ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] [API Change] New authentication method in QNetworkAccessManager
On 9 March 2014 20:13, Kurt Pattyn pattyn.k...@gmail.com wrote: On 09 Mar 2014, at 21:02, Giuseppe D'Angelo dange...@gmail.com wrote: On 9 March 2014 15:10, Kurt Pattyn pattyn.k...@gmail.com wrote: Also, the connection between the authenticationRequired signal and the slot must be a direct connection. IIRC SSL sockets had the same issue when SSL errors are raised. Has it been solved there? How? AFAIK, it has not been solved. The problem is the same. It has been partially solved, in recent versions you can set the socket to pause when ssl errors occur which avoids the need to use a nested event loop, see https://codereview.qt-project.org/#change,13834 We want this to be supported for authentication and also for ssl when there are no errors. It is being tracked as QTBUG-19032https://bugreports.qt-project.org/browse/QTBUG-19032 . Rich. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development