Bug 435442

Summary: drkonqi fails to login bugs.kde.org due to 'incorrect' password.
Product: [Applications] drkonqi Reporter: Lemuel Simon <lemuelsimon32>
Component: generalAssignee: Plasma Bugs List <plasma-bugs-null>
Status: RESOLVED FIXED    
Severity: normal CC: sitter
Priority: NOR Keywords: drkonqi
Version First Reported In: 5.21.3   
Target Milestone: ---   
Platform: openSUSE   
OS: Linux   
Latest Commit: Version Fixed/Implemented In: 5.21.5
Sentry Crash Report:

Description Lemuel Simon 2021-04-06 23:18:22 UTC
SUMMARY
When making a bug-report with drkonqi, it fails to login due to 'incorrect login or password'. But I can login with the same credentials on my browser (Firefox) just fine. The bug may be related to this: https://bugs.kde.org/show_bug.cgi?id=237026

STEPS TO REPRODUCE
1. Trigger drkonqi via application crash.
2. Enter login credentials for bugs.kde.org

OBSERVED RESULT
Login fails due to 'incorrect login or password'. 

EXPECTED RESULT
Login succeeds and bug-report is uploaded.

SOFTWARE/OS VERSIONS
Operating System: openSUSE Leap 15.2
KDE Plasma Version: 5.21.3
KDE Frameworks Version: 5.80.0
Qt Version: 5.15.2
Kernel Version: 5.3.18-lp152.66-default
OS Type: 64-bit
Graphics Platform: X11
Processors: 4 × Intel® Core™ i5-3320M CPU @ 2.60GHz
Memory: 7.6 GiB of RAM
Graphics Processor: Mesa DRI Intel® Ivybridge Mobile

ADDITIONAL INFORMATION
When I logged in via Firefox, I disabled the 'IP SESSION SECURITY FEATURE', before I attempted logging in with drkonqi. I've also logged out of the browser before I tried drkonqi.

Funnily enough, login works in drkonqi if change my password from a complex 'm&T9zSZ>0,q%FFDN' to something basic like 'abcd1234...'. <--- I have already changed the aforementioned passwords, don't worry :)

P.S: I'm a newbie, how should I include related bugs?
Comment 1 Harald Sitter 2021-04-07 12:19:34 UTC
Thanks for the example password. I can reproduce the problem and know what the problem is, a bit tricky to fix in a non-invasive manner though.

url encoding (or rather decoding before re-encoding) is too aggressive and decodes the characters after the percent sign.
Comment 2 Bug Janitor Service 2021-04-07 13:58:08 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/drkonqi/-/merge_requests/17
Comment 3 Harald Sitter 2021-04-14 09:52:20 UTC
Git commit d657b0e6023cac063d91a5efb36135f1b6ce7359 by Harald Sitter.
Committed on 14/04/2021 at 09:50.
Pushed by sitter into branch 'master'.

move bugz api from qurlquery to a custom container

it's a bit meh but we want to get aggressive percent encoding, qurlquery
doesn't give us that.

so what we had before was that one would feed QUQs into the API, then
the Connection would decode the QUQ to more comprehensively encode it
before constructing the final request QUrl that gets passed to KIO.
this had a number of entirely unwanted side effects the QUQ internally
partially recodes from user input and we'd have to select a decoding
choice when getting it out of QUQ. depending on the input that went
gloriously wrong

e.g. password with the verbatim part %FF would decode to unicode

similarly we can't simply use a more limited decoding set because it
couldn't possibly small enough

e.g. a password with the verbatim part %3C would decode to < even when
using DecodeReserved

to solve this properly we now have two distinct states for the queries
represented by the actual objects we deal with.

our Query object is literal input and never transcodes

just before passing to KIO our Query is translated to a QUrlQuery and as
part of that comprehensively encoded through toPercentEncoding. this
should then ensure that what KIO deals with is indeed encoded in a
similar fashion to how a browser would do it, and how bugz expects it.

to ensure this works correctly the connection test has been extended
accordingly
FIXED-IN: 5.21.5

M  +1    -0    src/bugzillaintegration/libbugzilla/CMakeLists.txt
M  +3    -3    src/bugzillaintegration/libbugzilla/autotests/attachmenttest.cpp
M  +3    -3    src/bugzillaintegration/libbugzilla/autotests/bugfieldtest.cpp
M  +3    -3    src/bugzillaintegration/libbugzilla/autotests/bugtest.cpp
M  +3    -3    src/bugzillaintegration/libbugzilla/autotests/bugzillatest.cpp
M  +3    -3    src/bugzillaintegration/libbugzilla/autotests/commenttest.cpp
M  +10   -3    src/bugzillaintegration/libbugzilla/autotests/connectiontest.cpp
M  +3    -3    src/bugzillaintegration/libbugzilla/autotests/producttest.cpp
M  +1    -1    src/bugzillaintegration/libbugzilla/bugzilla.cpp
M  +2    -2    src/bugzillaintegration/libbugzilla/clients/commands/bugsearch.cpp
M  +1    -1    src/bugzillaintegration/libbugzilla/clients/commands/bugsearch.h
M  +3    -3    src/bugzillaintegration/libbugzilla/clients/commands/querycommand.cpp
M  +3    -3    src/bugzillaintegration/libbugzilla/clients/commands/querycommand.h
M  +10   -10   src/bugzillaintegration/libbugzilla/connection.cpp
M  +10   -8    src/bugzillaintegration/libbugzilla/connection.h
A  +34   -0    src/bugzillaintegration/libbugzilla/query.cpp     [License: LGPL(3+eV) LGPL(v3.0) LGPL(v2.1)]
A  +38   -0    src/bugzillaintegration/libbugzilla/query.h     [License: LGPL(3+eV) LGPL(v3.0) LGPL(v2.1)]

https://invent.kde.org/plasma/drkonqi/commit/d657b0e6023cac063d91a5efb36135f1b6ce7359
Comment 4 Harald Sitter 2021-04-14 09:53:00 UTC
Git commit 7e02852def51c91ed64d525a999d798952786fc7 by Harald Sitter.
Committed on 14/04/2021 at 09:52.
Pushed by sitter into branch 'Plasma/5.21'.

move bugz api from qurlquery to a custom container

it's a bit meh but we want to get aggressive percent encoding, qurlquery
doesn't give us that.

so what we had before was that one would feed QUQs into the API, then
the Connection would decode the QUQ to more comprehensively encode it
before constructing the final request QUrl that gets passed to KIO.
this had a number of entirely unwanted side effects the QUQ internally
partially recodes from user input and we'd have to select a decoding
choice when getting it out of QUQ. depending on the input that went
gloriously wrong

e.g. password with the verbatim part %FF would decode to unicode

similarly we can't simply use a more limited decoding set because it
couldn't possibly small enough

e.g. a password with the verbatim part %3C would decode to < even when
using DecodeReserved

to solve this properly we now have two distinct states for the queries
represented by the actual objects we deal with.

our Query object is literal input and never transcodes

just before passing to KIO our Query is translated to a QUrlQuery and as
part of that comprehensively encoded through toPercentEncoding. this
should then ensure that what KIO deals with is indeed encoded in a
similar fashion to how a browser would do it, and how bugz expects it.

to ensure this works correctly the connection test has been extended
accordingly
FIXED-IN: 5.21.5


(cherry picked from commit d657b0e6023cac063d91a5efb36135f1b6ce7359)

M  +1    -0    src/bugzillaintegration/libbugzilla/CMakeLists.txt
M  +3    -3    src/bugzillaintegration/libbugzilla/autotests/attachmenttest.cpp
M  +3    -3    src/bugzillaintegration/libbugzilla/autotests/bugfieldtest.cpp
M  +3    -3    src/bugzillaintegration/libbugzilla/autotests/bugtest.cpp
M  +3    -3    src/bugzillaintegration/libbugzilla/autotests/bugzillatest.cpp
M  +3    -3    src/bugzillaintegration/libbugzilla/autotests/commenttest.cpp
M  +10   -3    src/bugzillaintegration/libbugzilla/autotests/connectiontest.cpp
M  +3    -3    src/bugzillaintegration/libbugzilla/autotests/producttest.cpp
M  +1    -1    src/bugzillaintegration/libbugzilla/bugzilla.cpp
M  +2    -2    src/bugzillaintegration/libbugzilla/clients/commands/bugsearch.cpp
M  +1    -1    src/bugzillaintegration/libbugzilla/clients/commands/bugsearch.h
M  +3    -3    src/bugzillaintegration/libbugzilla/clients/commands/querycommand.cpp
M  +3    -3    src/bugzillaintegration/libbugzilla/clients/commands/querycommand.h
M  +10   -10   src/bugzillaintegration/libbugzilla/connection.cpp
M  +10   -8    src/bugzillaintegration/libbugzilla/connection.h
A  +34   -0    src/bugzillaintegration/libbugzilla/query.cpp     [License: LGPL(3+eV) LGPL(v3.0) LGPL(v2.1)]
A  +38   -0    src/bugzillaintegration/libbugzilla/query.h     [License: LGPL(3+eV) LGPL(v3.0) LGPL(v2.1)]

https://invent.kde.org/plasma/drkonqi/commit/7e02852def51c91ed64d525a999d798952786fc7