> On Okt. 7, 2014, 1:13 nachm., Thomas Lübking wrote:
> > drkonqi/bugzillalib.cpp, line 81
> > <https://git.reviewboard.kde.org/r/120431/diff/4/?file=316623#file316623line81>
> >
> > The patch largely consists of hand-crafted version handling.
> >
> > replacing this by "int version = KDE_MAKE_VERSION(major, minor,
> > release);" and simple integer metricts for comparism should considerably
> > lower complexity (thus make the patch easier to be accepted ;-)
> >
> > Yes, I know it's crap to write a lot of code and remove it afterwards.
>
> Ian Wadham wrote:
> This is exactly the kind of advice I hope for in a review. I did look to
> see, several weeks ago, if there were some version-compare methods in KDE or
> Qt or on Google, but did not find any. I also considered something like
> version = (major * 1000000 + minor * 1000 + release), but thought it would be
> rather a 1960s style kludge... ;-) I have lived and worked through all the
> trouble those YYMMDD strings caused (Y2K and all that). Still, I suppose the
> parts of Bugzilla versions are unlikely to go past 255...
>
> I have no objection to deleting code and replacing it with something
> shorter and simpler. I guess I would still need the code to convert a QString
> version (from Bugzilla and the bugs.kde.org database) to a single
> KDE_MAKE_VERSION integer, or is there something else in KDE/Qt to do that?
>
> What *does* worry me is "5-minutes-to-midnight" re-programming, i.e. I
> would have to make and test all changes on Thursday, just hours before Albert
> starts tagging KDE 4.14.2. In my experience, that could be riskier than
> committing my patch as it stands.
>
> But either way, we at least have a month to fix any problems before KDE
> 4.14.3, the last release of KDE 4. Then there is always KDE 14.12 and Dr
> Konqi is an app, not a library...
>
> @Albert and Thomas: Please let me know what you would like me to commit
> on Thursday: my patch as it stands, my patch simplified as Thomas suggests or
> Frédéric's patch.
>
> Albert Astals Cid wrote:
> I would prefer the simplified version. If you really feel you're going to
> break the code, i'll take a commitment to do the simplified version for 4.14.3
> I guess I would still need the code to convert a QString version (from
> Bugzilla and the bugs.kde.org database) to a single KDE_MAKE_VERSION integer,
> or is there something else in KDE/Qt to do that?
You'll have to split the received string into 3 integers (major, minor,
release) for the KDE_MAKE_VERSION macro.
I found no spec for it. Is it really more than "1.2.3"?
In case: the present number splitting just collects the first 3 numbers - you'd
rather have to isolate "[0-9]*.[0-9]*.[0-9]*" first.
If not:
```cpp
QStringList l = string.split(QLatin1Char('1'));
if (l.count() == 3)
version = KDE_MAKE_VERSION(l.at(0).toUInt(), l.at(1).toUInt(),
l.at(2).toUInt());
else
kDebug() << "sth's severely fucked up here";
fi
- Thomas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120431/#review68051
-----------------------------------------------------------
On Okt. 7, 2014, 7:42 vorm., Ian Wadham wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120431/
> -----------------------------------------------------------
>
> (Updated Okt. 7, 2014, 7:42 vorm.)
>
>
> Review request for KDE Software on Mac OS X, KDE Runtime, Ben Cooksley, Darío
> Andrés Rodríguez, George Kiagiadakis, Jekyll Wu, and Matthias Fuchs.
>
>
> Bugs: 337742
> http://bugs.kde.org/show_bug.cgi?id=337742
>
>
> Repository: kde-runtime
>
>
> Description
> -------
>
> When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security
> method used by Bugzilla changed from cookies to tokens that had to be
> supplied as parameters with every secure remote-procedure call. Further
> changes to security methods have been announced by Bugzilla and are
> documented for unstable 4.5.x versions of Bugzilla software. Tokens will be
> deprecated and then discontinued. When this happens, Dr Konqi will need to
> supply a user-login name and a password with every secure remote-procedure
> call. Furthermore, the traditional "User.login" call presently used by Dr
> Konqi will be deprecated and discontinued.
>
> This patch fixes the tokens problem, which has given rise to several bug
> reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also
> provides for automatic switching to passwords-only security as and when the
> Bugzilla version changes again. This uses
> a general data-driven approach which can be easily updated, ahead of time,
> next time Bugzilla announces a change that affects Dr Konqi, whether it be in
> security methods or some other feature.
>
> NOTES:
> 1. This patch is intended to be forward-portable to Frameworks/KF5, but I
> work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do
> the porting and testing. So could someone else please do it?
> 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses
> the tokens issue only, but it should be reviewed and shipped as a matter of
> urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE
> 4.14 being due for tagging on Thursday, 9 October. That will leave more time
> for this review (120431) of my more long-term and more general patch.
> 3. The passwords-only part of my patch is currently storing the password in
> clear. Suggestions re encryption are welcomed --- or the code could be
> changed to make use of KWalletD mandatory (but that might not be fully
> portable to all platforms).
> 4. When the Bugzilla call "User.login" is discontinued, some re-sequencing of
> the flow of KAssistantDialog pages will be needed. I have not attempted to do
> that at this stage. Probably the entry of the user name and password should
> be delayed until the report has been accepted by the Dr Konqi logic and it is
> just about to be sent to bugs.kde.org or attached to an existing bug report.
>
> REFERENCES:
> http://www.bugzilla.org/docs/
> http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService.html#LOGGING_IN
> Bugzilla 4.5.x (future) API doco re security
> http://www.bugzilla.org/docs/4.4/en/html/api/Bugzilla/WebService.html#LOGGING_IN
> Bugzilla 4.4.5 (current) API doco re security
> http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/User.html#login
> User.login will be DEPRECATED in 4.5.x
>
>
> Diffs
> -----
>
> drkonqi/bugzillalib.h 570169b
> drkonqi/bugzillalib.cpp f74753c
> drkonqi/reportassistantpages_bugzilla.h b7af5b8
> drkonqi/reportassistantpages_bugzilla.cpp 22183f0
>
> Diff: https://git.reviewboard.kde.org/r/120431/diff/
>
>
> Testing
> -------
>
> Used the bugstest.kde.org database and KDE 4 master on KDE/kde-runtime
> repository.
>
> Tested a range of version numbers (see commented-out test data) against a
> range of 5 or 6 hypothetical and real Bugzilla versions at which things could
> or will change. This was to test the basic version-checking and
> feature-choosing algorithm.
>
> Tested submitting both full reports and attached reports, using both the
> token method and the passwords-only method.
>
> Also tested with KWalletD supplying the username and password on Dr Konqi's
> login dialog.
>
>
> Thanks,
>
> Ian Wadham
>
>