Re: [Development] [API Change] New authentication method in QNetworkAccessManager

2014-03-10 Thread Kurt Pattyn

 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

2014-03-10 Thread Giuseppe D'Angelo
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

2014-03-10 Thread Kurt Pattyn


 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 Thread Konstantin Ritt
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

2014-03-09 Thread Kevin Krammer
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

2014-03-09 Thread Kurt Pattyn

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

2014-03-09 Thread Olivier Goffart
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

2014-03-09 Thread Giuseppe D'Angelo
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

2014-03-09 Thread Kurt Pattyn

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

2014-03-09 Thread Kurt Pattyn

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

2014-03-09 Thread Richard Moore
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