Re: Banning QNetworkAccessManager

2020-02-02 Thread Volker Krause
I agree on the problem of QNAM's default, see also https://conf.kde.org/en/
akademy2019/public/events/135 on that subject.

On Saturday, 1 February 2020 23:24:14 CET Ben Cooksley wrote:
[...]
> Prior to now, i've taken the approach of advertising that
> QNetworkAccessManager is broken and needs a flag set within
> applications when used, however unfortunately this seems to have been
> an ineffective approach and cases of broken behaviour continue to
> appear (despite this brokeness being known about and advertised since
> back in the Qt 4.x series when this class was introduced)
> 
> Therefore, given the Qt Project is unlikely to change their position
> on this, I would like to propose the following:

The Qt Project is actually in favor of changing at least the redirection 
default to exactly what we want it to be.
https://codereview.qt-project.org/c/qt/qtbase/+/273175 has the change, and got 
approval both by Lars and one of the maintainers. It is merely stuck in 
dealing with the unit test fallout in Qt's CI that somebody has to take care 
of. Doesn't immediately help us of course, but claiming Qt is unwilling to 
change this is simply wrong.

> 1) That effective immediately, QNetworkAccessManager and it's children
> classes be banned and prohibited within KDE software, to be enforced
> by server side hooks.
> 2) That we fork QNetworkAccessManager and the associated classes
> within the appropriate Framework (to be determined), where the
> defective behaviour can then be corrected.

While this might solve the redirection problem, it brings us yet another then 
unmaintained HTTP implementation next to the one in KIO, which is a far bigger 
problem long term. We need less of those, not more.

To address the problem of people not using the correct defaults, I did propose 
a much less extreme approach in the above mentioned talk at Akademy, namely 
having a KNetworkAccessManager as a sub-class of QNAM in a low-tier frameworks 
that essentially just enables redirects and HSTS. QNAM's implementation isn't 
the problem after all, the defaults are.

Commit hooks warning about QNAM usage might still be a good idea, but as a 
warning only. Hard blocking QNAM-using code would block at least three repos I 
spend a lot of time on currently, none of which even talks to KDE servers.

If you need to take more drastic measures against code not doing this 
correctly regardless, you can do that my dropping the server-side workarounds. 
Yes, that would break the affected application possibly, but at least it would 
not cause massive collateral damage for the people using this correctly.

It would also help to know where specifically we have that problem, so we can 
actually solve it, and so we can figure out why we failed to fix this there 
earlier.

Regards,
Volker


signature.asc
Description: This is a digitally signed message part.


Fixing QNetworkAccessManager use for KDE services

2020-02-02 Thread Friedrich W. H. Kossebau
Hi Ben,

sorry to hear about this pain you have in all the good work you do that allows 
us to enjoy the high reliability of the KDE services. I would like to help to 
reduce that pain.

Am Samstag, 1. Februar 2020, 23:24:14 CET schrieb Ben Cooksley:
> Hi all,
> 
> For an extremely long time now, it has been a known issue with the
> QNetworkAccessManager that by default it does not follow redirects as
> issued by the server it is accessing. This is something the Qt Project
> has refused to address using the justification of 'behavioural
> compatibility'

This justification makes sense to me. People who have had in their code manual 
redirect handling would not be happy if Qt suddenly starts to do things 
internally in potentially other ways.

Also are they announcing in the API dox to consider changing the default:
"For backwards compatibility the default value is 
QNetworkRequest::ManualRedirectPolicy. This may change in the future and some 
type of auto-redirect policy will become the default; clients relying on 
manual redirect handling are encouraged to set this policy explicitly in their 
code."
https://doc.qt.io/qt-5/qnetworkaccessmanager.html#setRedirectPolicy

> This behaviour is contrary to that of just about every other HTTP
> stack (with the exception of libcurl from my understanding) and is
> behaviour that nobody expects.

In my case it would be: nobody thought about.
When talking to a given hardcoded address, e.g. to query a data blob, and it 
no longer resolves I would rather expect by default that the service is no 
longer existing. Mentally driven by C++ ABI concepts that method names & 
signatures have to be stable ;)
Possibly admins/web service developers might think different, as they might 
like to be flexible under which urls to respond to requests, given redirects 
exists in the protocol and thus invite to be used.
Might be a clash of cultures to some degree.

> As a consequence of this (broken) behaviour, Sysadmin has been
> effectively forced to implement workarounds and other compatibility
> measures in place to keep applications functional.

It is also the consequence of no developers having picked up the new built-in 
redirect accepting options having appeared in Qt 5.6 or 5.9 (more control).
And they have not done so because they (at least me) have not been aware that 
KDE sysadmins would like to be flexible when it comes to data/web services on 
KDE servers and the addresses under which they are available. See below for 
proposal how to fix that.

[...]

> Therefore, given the Qt Project is unlikely to change their position
> on this, I would like to propose the following:
> 1) That effective immediately, QNetworkAccessManager and it's children
> classes be banned and prohibited within KDE software, to be enforced
> by server side hooks.
> 2) That we fork QNetworkAccessManager and the associated classes
> within the appropriate Framework (to be determined), where the
> defective behaviour can then be corrected.

I cannot see how both help with released code out there already in the wild. 
To prepare future released code to be supportive to your redirecting desires 
when it comes to KDE services, I would rather propose this:

A) Document the need to enable redirects in requests when using KDE server 
services in the coding policies as well in the documentation of the respective 
KDE services, including code examples how to write those.
B) Have a rally to fix all current code.
C) Have an issue on bugreports.qt.io to see that Qt 6 will have changed the 
default, to what web service developers/admins would prefer.

E.g. in KDevelop code there is a query for https://projects.kde.org/
kde_projects.xml. What kind of redirect support do KDE server admins expect to 
be supported? So what QNetworkRequest::RedirectPolicy value should be set? 
What QNetworkRequest::maximumRedirectsAllowed?
Ideally one could find answers to these questions on community.kde.org.

Cheers
Friedrich