Fatal error when password starts with "-"

[ComvationAG]

Bug

• How to reproduce the issue?
  git ftp catchup --branch master --user test --passwd "-test"
  fatal: Too few arguments for option -p.

• Can you point to a Git repository to reproduce it?
  → Happen on every git repo.

• Or can you describe the directory structure?
  → See previous comment .

Environment

/bin/ash
/bin/ash
Linux 52930a91fe2d 4.9.93-linuxkit-aufs #1 SMP Wed Jun 6 16:55:56 UTC 2018 x86_64 Linux
git-ftp version 1.5.1

[maciejkopecpl]

@ComvationAG I had the same issue, as a workaround I used command: git config git-ftp.password -PASS

[LukasFritzeDev]

Does it work if you use --ask-passwd instead of --passwd?

[LukasFritzeDev]

I've now had time to get to the root of this issue.

This issue is caused by the processing of options in the script. We always have to check if the next argument is the next option or the value of the current one. This is done for every every option in the same way, in this case it’s done here:

git-ftp/git-ftp

Lines 1546 to 1551 in d496de9

	if ! echo "$2" | egrep -q '^-'; then
	REMOTE_PASSWD="$2"
	shift
	else
	print_error_and_die "Too few arguments for option -p." "$ERROR_MISSING_ARGUMENTS"
	fi

So it does not matter if the value is quoted or not, it is passed to this check. And this check fails because the value starts with a -. It is working if -P, --ask-passwd is used or the password is read from the config, because it kind of bypasses this processing.

I don’t know of a way to check if the parameter was originally quoted or not. But I can think of other solutions:

1. Do not check for - just for this option
   If the option -p, --passwd is set there must always be a value given. So we could just say: what ever comes next, treat it as the value for password. Of course in this case – if omitting the value – any valid option coming next would be treated as the password and it would be hard to catch this mistake.
   The fix would look like this:

REMOTE_PASSWD="$2"
shift

2. Add a custom escape sequence
   We could define a custom escape sequence for this special case. At the moment if the password is set like -p "\-test" it is passed in exactly this way to curl, so curl uses \-test as PASS. But if the value begins with \- we could remove the \ and only use -. This would fix this and makes sure the password is set to -test in this case, but it would lead to other problems:
   1. We can not make a difference between single quotes ('), double quotes (") or none quotes at all. I don’t know if this really is a problem but a least it’s not the convention to evaluate escape sequences in single quoted string, is it?
   2. What happens if the password really starts with the sequence \-? Then we have fixed the case for a password starting with - but added another incompatible password. How often does one or the other case occur?

if ! echo "$2" | egrep -q '^-'; then
	if echo "$2" | egrep -q '^\\-'; then
		REMOTE_PASSWD="${2:1}"
	else 
		REMOTE_PASSWD="$2"
	fi
	shift
else
	print_error_and_die "Too few arguments for option -p." "$ERROR_MISSING_ARGUMENTS"
fi

3. No fix but documentation
   Not really a solution but a way of dealing with this limitation. Instead of fixing this and adding other issues with the fix we could define this as a special case where you have to use git config, --ask-passwd or ~/.netrc to set the password.

What do you think? What’s the best way to go?

[ComvationAG]

Hi @dertanzschuh

Thank you for this detailed feedback.
I prefer solution 3, because the other solutions can produce other weird problems, that you described.

My only addition is, that you in the error message ("Too few arguments for option -p ..") a direct link to the documentation part, where the people get informed about limit of a password that starts with '-'.

Thank you for your support.

[LukasFritzeDev]

@ComvationAG you‘re welcome.

My only addition is, that you in the error message ("Too few arguments for option -p ..") a direct link to the documentation part, where the people get informed about limit of a password that starts with '-'.

Yes that’s a good idea. Thank you for this.

[avengerx]

This issue is actually fixed, where you can just use:

git ftp catchup --branch=master --user=test --passwd="-test"

In this case not even quotes are needed...

git ftp catchup --branch=master --user=test --passwd=-test

I am looking up history, and the tool supported this already back when this issue was raised, not sure why nobody tried this back then.

The --remote-root is the only flag taking an argument that does not support this syntax. If you look at other git commands (git-log for one), their man pages suggests the preferred syntax is splitting flag arguments with the equals symbol.

[LukasFritzeDev]

Sometimes you don't think of the simplest solutions. Thanks a lot for your addition 😃