> On oct. 7, 2014, 1:13 p.m., 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.
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 - Albert ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/#review68051 ----------------------------------------------------------- On oct. 7, 2014, 7:42 a.m., Ian Wadham wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120431/ > ----------------------------------------------------------- > > (Updated oct. 7, 2014, 7:42 a.m.) > > > 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 > >
