Fix URL matching with Browser Integration #3759
Conversation
|
Should we alert the user some how that there is an invalid url entered? Perhaps a simple label that pops up or using the red x icon in the url field similar to when passwords are mismatched. We would have to exclude cmd:// style URLs from that check though. |
|
@droidmonkey I agree. Some kind of warning is needed in the GUI also. |
|
Something like "the entered URL will not work with the browser extension because {reason}" |
|
Will this PR also fix handling of URLs with trailing space ( Example: entry with an URL like Further observations:
|
|
@Talv It fixes those too. Just tested it. |
|
This PR has still some issues. Trying to fix them and add the GUI warning. Then we are good to go. |
|
Problems with the matching are fixed. I'll do the UI stuff next. |
|
Added the UI warning. This is good to go! |
|
This isn't ready for prime time. It's marking perfectly valid URL's as invalid. It also doesn't mark invalid URL's in the new browser settings page.
|
|
I am going to withhold this until 2.5.2 |
|
@varjolintu can you separate the fixes for the URL Parsing from the UI error checking code? I do want the URL parsing to be fixed in 2.5.1. |
|
@droidmonkey Sure, I can separate the UI fixes for now. EDIT: Done. |
varjolintu
force-pushed
the
hotfix/browser_url_validation
branch
from
3cdc3f7 to
141e7e7
Compare
|
@droidmonkey One more thing. Should URL's without a scheme match every time with all scheme's in the page URL? Or only when the scheme matching is disabled? EDIT: I'm in favor to fallback to |
|
Yah if there is no scheme we assume https |
varjolintu
force-pushed
the
hotfix/browser_url_validation
branch
from
141e7e7 to
ac79177
Compare
|
Pushed and rebased. |
|
@phoerious We are merging this with 2.5.1, but the UI stuff will be another PR for 2.5.2. That's why I changed the milestone. |
|
ok. |
Added - Add programmatic use of the EntrySearcher [#3760] - Explicitly clear database memory upon locking even if the object is not deleted immediately [#3824] - macOS: Add ability to perform notarization of built package [#3827] Changed - Reduce file hash checking to every 30 seconds to correct performance issues [#3724] - Correct formatting of notes in entry preview widget [#3727] - Improve performance and UX of database statistics page [#3780] - Improve interface for key file selection to discourage use of the database file [#3807] - Hide Auto-Type sequences column when not needed [#3794] - macOS: Revert back to using Carbon API for hotkey detection [#3794] - CLI: Do not show protected fields by default [#3710] Fixed - Secret Service: Correct issues interfacing with various applications [#3761] - Fix building without additional features [#3693] - Fix handling TOTP secret keys that require padding [#3764] - Fix database unlock dialog password field focus [#3764] - Correctly label open databases as locked on launch [#3764] - Prevent infinite recursion when two databases AutoOpen each other [#3764] - Browser: Fix incorrect matching of invalid URLs [#3759] - Properly stylize the application name on Linux [#3775] - Show application icon on Plasma Wayland sessions [#3777] - macOS: Check for Auto-Type permissions on use instead of at launch [#3794]
|
It seems that with the "fix" only first level domain is considered. |
|
@a-v-popov Yes, that's what the next PR will fix. |
Type of change
Description and Context
Ignores junk URL's, and fixes port matching. Also renames some functions and variables.
If scheme matching is enabled, entry URL's without a scheme will fallback to
https.Fixes #3751.
Testing strategy
Manually. Previously an entry with URL
https:///example.comwas requested for all sites. Also, tests are included.Checklist:
-DWITH_ASAN=ON. [REQUIRED]