Handle URL port and scheme when requesting credentials#2003
Handle URL port and scheme when requesting credentials#2003droidmonkey merged 1 commit intokeepassxreboot:release/2.3.4from
Conversation
phoerious
left a comment
There was a problem hiding this comment.
Looks good, but what about HTTPS vs HTTP?
|
That could be done too. I'll make the fixes to this PR. |
b1701f6 to
1701bcb
Compare
|
At the same time I was looking an old issue #1340. I'm not sure if it's wise to keep the old compare method where URL is compared to the entry title. I think it should only do the compare to the URL. |
droidmonkey
left a comment
There was a problem hiding this comment.
This PR conflicts with #2055. I also do not like the way this is implemented, there are a ton of local variables created that don't seem to add to the overall objective.
src/browser/BrowserService.cpp
Outdated
| if ((!entryTitle.isEmpty() && hostname.contains(entryTitle)) | ||
| || (!entryUrl.isEmpty() && hostname.contains(entryUrl)) | ||
| || (matchUrlScheme(entryTitle) && hostname.endsWith(QUrl(entryTitle).host())) | ||
| || (matchUrlScheme(entryUrl) && hostname.endsWith(QUrl(entryUrl).host())) ) { |
There was a problem hiding this comment.
Why not just use "entryQUrl" that you defined above? This conditional is next to impossible to decipher.
There was a problem hiding this comment.
Use it for what? I'm not sure what you mean.
There was a problem hiding this comment.
You recreate entryQUrl within the conditional when you do QUrl(entryUrl)
e8d9be1 to
3d82da4
Compare
|
Conflict resolved. |
src/browser/BrowserService.cpp
Outdated
| if ((!url.isEmpty() && hostname.contains(url)) | ||
| || (matchUrlScheme(url) && hostname.endsWith(QUrl(url).host()))) { | ||
| if ((!entryUrl.isEmpty() && hostname.contains(entryUrl)) | ||
| || (matchUrlScheme(entryUrl) && hostname.endsWith(QUrl(entryUrl).host()))) { |
There was a problem hiding this comment.
You're still recreating entryQUrl here
868be21 to
13f31b6
Compare
- Show all URL schemes in entry view [#1768] - Disable merge when database is locked [#1975] - Fix intermittent crashes with favorite icon downloads [#1980] - Provide potential crash warning to Qt 5.5.x users [#2211] - Disable apply button when creating new entry/group to prevent data loss [#2204] - Allow for 12 hour timeout to lock idle database [#2173] - Multiple SSH Agent fixes [#1981, #2117] - Multiple Browser Integration enhancements [#1993, #2003, #2055, #2116, #2159, #2174, #2185] - Fix browser proxy application not closing properly [#2142] - Add real names and Patreon supporters to about dialog [#2214] - Add settings button to toolbar, Donate button, and Report a Bug button to help menu [#2214] - Enhancements to release-tool to appsign intermediate build products [#2101]
Handle URL port and scheme when requesting credentials.
Description
Entries with different port or scheme in URL are ignored. If an entry has a
httpscheme, credentials are not received when accessing a site withhttps.Motivation and context
Fixes keepassxreboot/keepassxc-browser#143.
Fixes #1467.
How has this been tested?
Manually.
Types of changes
Checklist:
-DWITH_ASAN=ON. [REQUIRED]