Ticket #3869: actually use the .netrc password for ftp vfs

[Vrihub]

Proposed changes

This fixes #3869, when one wants to log into an ftp server only providing username and hostname, and relies on mc to retrieve the password from the .netrc file. Without this, mc retrieves the password from the .netrc file, but doesn't actually use it to login, and asks the user for a password instead.

Please review, this works for me, but I'm not 100% sure it's the best solution.

• Resolves: ftp: fail to retrieve password from .netrc when user is in URL #3869

Checklist

👉 Our coding style can be found here: https://midnight-commander.org/coding-style/ 👈

• I have referenced the issue(s) resolved by this PR (if any)
• I have signed-off my contribution with git commit --amend -s
• Lint and unit tests pass locally with my changes (make indent && make check)
• I have added tests that prove my fix is effective or that my feature works
• I have added the necessary documentation (if appropriate)

[mc-worker]

There is no reason to duplicate new_passwd that is just free'd below.

[zyv]

Well, I think this is the safest and easiest way to code that, isn't it? What would be your way of doing it?

I'm more concerned about this introducing a logical mistake. I think this if-block should go above the previous one. Otherwise, it just reverts the effect of the previous condition, but not only for the same user but also for a different user.

[Vrihub]

You're right, there's a logical error, but I think the correct way is using else, see my latest fixup commit. I've tested the following cases and mc behaves as expected:

• ftp://host: uses user and passwd from .netrc
• ftp://user@host: uses user's passwd from .netrc
• ftp://user_not_in_netrc@host: asks for user_not_in_netrc's password

Please just confirm that it's ok to use g_strdup(), I'm not sure about that, I've used it in analogy with line 364 path_element->user = g_strdup ("anonymous");

[Vrihub]

Please just confirm that it's ok to use g_strdup(), I'm not sure about that, I've used it in analogy with line 364 path_element->user = g_strdup ("anonymous");

Well, if I just do path_element->password = new_passwd;, the ftp login fails, and mc segfaults.

[mc-worker]

Well, if I just do path_element->password = new_passwd;, the ftp login fails, and mc segfaults.

Because of use-after-free. You can take new_passwd in the one branch and free it in the other one.

[zyv]

I actually think that branching free is less safe than duplicating the string. The performance penalty of a few microseconds is negligible, because it isn't in a loop, but if later more code is added, whether to free or not has to be correctly considered in all branches. As we can see, we have a lot of bugs creeping in due to subtle errors in conditions. Using g_strdup always, even if it's not really needed, is more like ownership transfer semantics in C++. So it's harder to make a mistake when the code is modified later.

[mc-worker]

I actually think that branching free is less safe than duplicating the string. The performance penalty of a few microseconds is negligible, because it isn't in a loop, but if later more code is added, whether to free or not has to be correctly considered in all branches. As we can see, we have a lot of bugs creeping in due to subtle errors in conditions. Using g_strdup always, even if it's not really needed, is more like ownership transfer semantics in C++. So it's harder to make a mistake when the code is modified later.

I disagree.

[zyv]

So how do you want to have it? Like in the first or second fixup?

[zyv]

/rebase

[zyv]

@mc-worker ok?