[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-frameworks-devel
Subject:    Re: Banning QNetworkAccessManager
From:       Johnny Jazeix <jazeix () gmail ! com>
Date:       2020-02-19 12:38:55
Message-ID: CAEtcAPFiMTfJWu7BYcTk3EOfsWwvvam7jpwMqCK_hs=u1Q6GTQ () mail ! gmail ! com
[Download RAW message or body]

Hi,

as Volker said, without any alternative, we can't just ban QNAM.
And even with one, we would need time to implement it (for example,
for GCompris, this part of the code was written by someone who is less
active now, so rewriting it, testing it and be sure it works will take
some time).

Can't we use lxr to find all applications using it, check if they use
it in a good way or not instead?

Johnny


Le mer. 19 f=C3=A9vr. 2020 =C3=A0 10:05, Ben Cooksley <bcooksley@kde.org> a=
 =C3=A9crit :
>
> On Wed, Feb 19, 2020 at 9:30 PM Volker Krause <vkrause@kde.org> wrote:
> >
> > On Wednesday, 19 February 2020 08:05:01 CET Ben Cooksley wrote:
> > > On Mon, Feb 3, 2020 at 7:42 AM Volker Krause <vkrause@kde.org> wrote:
> > > > 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 s=
ince
> > > > > back in the Qt 4.x series when this class was introduced)
> > > > >
> > > > > Therefore, given the Qt Project is unlikely to change their posit=
ion
> > > >
> > > > > on this, I would like to propose the following:
> > > > The Qt Project is actually in favor of changing at least the redire=
ction
> > > > default to exactly what we want it to be.
> > > > https://codereview.qt-project.org/c/qt/qtbase/+/273175 has the chan=
ge, 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 Q=
t is
> > > > unwilling to change this is simply wrong.
> > > >
> > > > > 1) That effective immediately, QNetworkAccessManager and it's chi=
ldren
> > > > > classes be banned and prohibited within KDE software, to be enfor=
ced
> > > > > 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 an=
other
> > > > 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 QN=
AM in
> > > > a low-tier frameworks that essentially just enables redirects and H=
STS.
> > > > QNAM's implementation isn't the problem after all, the defaults are=
.
> > > >
> > > > Commit hooks warning about QNAM usage might still be a good idea, b=
ut as a
> > > > warning only. Hard blocking QNAM-using code would block at least th=
ree
> > > > 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 th=
is
> > > > correctly regardless, you can do that my dropping the server-side
> > > > workarounds. Yes, that would break the affected application possibl=
y, but
> > > > at least it would not cause massive collateral damage for the peopl=
e
> > > > 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 fi=
x this
> > > > there earlier.
> > >
> > > Just bringing this up again - it seems we've not had much movement on
> > > this aside from the Wiki page.
> > >
> > > Examining that Qt merge request, it not only is targeted at just Qt
> > > 6.x, it also hasn't had any movement since the start of October 2019 =
-
> > > 4 months ago.
> > > Given that we are going to be on Qt 5.x for some time, that merge
> > > request can't be considered to be the 'fix' for this issue.
> >
> > The targeting of Qt6 is due to this aiming at dev when that was still Q=
t 5.x,
> > but you are right of course, somebody needs to do the work there to dri=
ve this
> > to completion, no matter in which version it will actually land.
>
> Indeed.
>
> >
> > > We need a solution that will ensure software released today doesn't
> > > cause us pain in several years time, because as I illustrated in an
> > > earlier email to this thread, software has a habit of remaining in us=
e
> > > for an extremely long amount of time (August 2014 being the release
> > > date of the Marble versions in question hitting that old URL).
> > >
> > > The easiest path forward is to simply ban QNetworkAccessManager.
> >
> > That might be the easiest path for you, but certainly not for me, given=
 two of
> > the modules I work on most atm are using QNAM (not even to talk to KDE
> > infrastructure), and would suddenly no longer be allowed to be hosted h=
ere?
>
> Ideally they would move to whatever replacement we have for QNAM
> should it be banned :)
>
> >
> > Also, I have yet to see a suggestion for a viable alternative to QNAM. =
The
> > initially proposed fork would mean trading the possibility of broken re=
direct
> > handling for an unmaintained HTTP stack, I don't think that's a good de=
al for
> > the security of our users.
> >
>
> It isn't perfect yes.
>
> Sadly however the negative user experience created by countless broken
> applications due to the Qt Project being uncooperative in this matter
> for many years doesn't really give us much in the way of other
> options.
>
> It doesn't help when Developers come to us asking for temporary
> measures to be put in place to keep their applications alive as
> they're getting bug reports about broken functionality. Saying no
> saves us the effort of maintaining the fixes, but leaves applications
> broken - which is not a fun position to be in as the one making the
> decision.
>
> > Instead, how about the way we approached this for KUserFeedback? Before
> > getting things deployed on KDE infrastructure for use by library or
> > application code, sysadmins and developers review if the implementation=
 is
> > allowing easy and secure operation of the infrastructure, redirect hand=
ling
> > being one part of this. In case of KUserFeedback you suggested protocol
> > changes, they were implemented, and automatic tests for a number of pos=
sible
> > redirect scenarios were added. Worked out fine IMHO, despite QNAM (the =
needed
> > changes weren't due to missing redirect handling, but due to somewhat n=
on-
> > standard redirects on HTTP POST).
> >
>
> Having a Sysadmin review an implementation that will be talking to a
> server is in general always a good idea, even outside of this
> particular usecase.
> It certainly helps protect against this class of issues that is for sure.
>
> It isn't a perfect defense though, as if the review isn't requested
> issues could slip past :(
>
> > There is a risk that the code breaks this again over time, independent =
of test
> > coverage, but I'd say that's not sysadmin's problem then, it's due to
> > application code not following the rules.
> >
> > This of course only helps with new infrastructure deployments, but so w=
ould a
> > QNAM ban only help with new releases.
> >
> > Additionally, improved documentation, a possible KNAM and/or driving th=
e QNAM
> > changes upstream can still be done alongside this obviously.
>
> Something like KNAM would allow for a more refined ban yes (allowing
> QNAM in the repository that hosts KNAM, but blocking it elsewhere)
>
> >
> > Regards,
> > Volker
>
> Cheers,
> Ben
[prev in list] [next in list] [prev in thread] [next in thread]