* Re: [pve-devel] [PATCH pve_flutter_frontend] Fix #6231: Removed whitespace-trimming from password entry field
2025-03-11 11:52 [pve-devel] [PATCH pve_flutter_frontend] Fix #6231: Removed whitespace-trimming from password entry field Alexander Abraham
@ 2025-03-13 8:19 ` Laurențiu Leahu-Vlăducu
2025-03-13 9:05 ` Friedrich Weber
1 sibling, 0 replies; 3+ messages in thread
From: Laurențiu Leahu-Vlăducu @ 2025-03-13 8:19 UTC (permalink / raw)
To: pve-devel
I can confirm that logging in with passwords containing leading or
trailing whitespaces did not work previously, and your patch fixes the
issue.
However, please note that pve_flutter_frontend is the wrong repository,
as this patch is meant to be applied on the proxmox_login_manager
repository instead.
Other than that, please consider:
Reviewed-by: Laurențiu Leahu-Vlăducu <l.leahu-vladucu@proxmox.com>
Tested-by: Laurențiu Leahu-Vlăducu <l.leahu-vladucu@proxmox.com>
On 11.03.25 12:52, Alexander Abraham wrote:
> A user reported a bug where they were attempting to login into our
> app for PVE and they used a password with two spaces at the end.
> The login failed because of these trailing spaces. I removed the
> string trimming for the password entry field during login so users can
> use passwords that have spaces inside them.
>
> Signed-off-by: Alexander Abraham <a.abraham@proxmox.com>
> ---
> lib/proxmox_login_form.dart | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/proxmox_login_form.dart b/lib/proxmox_login_form.dart
> index 735bd42..5916563 100644
> --- a/lib/proxmox_login_form.dart
> +++ b/lib/proxmox_login_form.dart
> @@ -464,7 +464,7 @@ class _ProxmoxLoginPageState extends State<ProxmoxLoginPage> {
> //cleaned form fields
> final origin = normalizeUrl(_originController.text.trim());
> final username = _usernameController.text.trim();
> - final String enteredPassword = _passwordController.text.trim();
> + final String enteredPassword = _passwordController.text;
> final String? savedPassword = widget.password;
>
> final password = ticket.isNotEmpty ? ticket : enteredPassword;
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [pve-devel] [PATCH pve_flutter_frontend] Fix #6231: Removed whitespace-trimming from password entry field
2025-03-11 11:52 [pve-devel] [PATCH pve_flutter_frontend] Fix #6231: Removed whitespace-trimming from password entry field Alexander Abraham
2025-03-13 8:19 ` Laurențiu Leahu-Vlăducu
@ 2025-03-13 9:05 ` Friedrich Weber
1 sibling, 0 replies; 3+ messages in thread
From: Friedrich Weber @ 2025-03-13 9:05 UTC (permalink / raw)
To: Proxmox VE development discussion, Alexander Abraham
Hi, thanks for the patch! I can't comment on the change itself, but have
some remarks on the commit message:
On 11/03/2025 12:52, Alexander Abraham wrote:
> A user reported a bug where they were attempting to login into our
> app for PVE and they used a password with two spaces at the end.
> The login failed because of these trailing spaces. I removed the
Note that commit messages should be written in imperative mood, see [1]
-- so the "I removed" should be rephrased to something like "Remove the
string trimming [...]".
Same for the first line of the comment message ("Removed").
If possible, it is also be nice to have a tag prefix in the first line
that describes the involved subsystem (as described in [1]). What I
usually do to get an idea is to check previous commits that touch the
same file(s) -- proxmox_login_form.dart (via `git log
proxmox_login_form.dart` or the git UI [2]) in your case. Here we're
lucky and there are several commits that use "login:" or "login form:",
[2] so either of them sound good. But of course this approach doesn't
always work, so sometimes I'll have to come up with something on my own.
Finally, and I realize this is not in [1] so we're entering personal
preference territory, but I still want to mention it: Most first lines
of commit messages in our projects start in lowercase, so you can
consider adapting that convention too.
So all in all, a nice first line might be
fix #6231: login form: remove whitespace trimming from password entry
or something like that.
I realize these suggestions may sound a little pedantic :) But in the
long run, sticking to the guidelines makes maintenance of our projects
easier. For example, if all commit messages are formulated in similar
style, one can skim commit logs faster when looking for bugs or new feature.
One more thing: When you send an initial patch (series) for a bug
report, it can also be nice to drop a (lore.proxmox.com) link in the bug
report for reference, so users can take a look at the series if they want.
> string trimming for the password entry field during login so users can
> use passwords that have spaces inside them.
>
> Signed-off-by: Alexander Abraham <a.abraham@proxmox.com>
> ---
> lib/proxmox_login_form.dart | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/proxmox_login_form.dart b/lib/proxmox_login_form.dart
> index 735bd42..5916563 100644
> --- a/lib/proxmox_login_form.dart
> +++ b/lib/proxmox_login_form.dart
> @@ -464,7 +464,7 @@ class _ProxmoxLoginPageState extends State<ProxmoxLoginPage> {
> //cleaned form fields
> final origin = normalizeUrl(_originController.text.trim());
> final username = _usernameController.text.trim();
> - final String enteredPassword = _passwordController.text.trim();
> + final String enteredPassword = _passwordController.text;
> final String? savedPassword = widget.password;
>
> final password = ticket.isNotEmpty ? ticket : enteredPassword;
[1]
https://pve.proxmox.com/wiki/Developer_Documentation#Commits_and_Commit_Messages
[2]
https://git.proxmox.com/?p=flutter/proxmox_login_manager.git;a=history;f=lib/proxmox_login_form.dart;h=735bd42d7c493aa8e39177b28963db00c6c29ebc;hb=HEAD
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 3+ messages in thread