Fix some CMP related msan failures

[mattcaswell]

The cmp_cli test is failing in some travis builds on master due to msan failures. There are two separate problems fixed by this PR:

1. Ensure a string is properly terminated in http_client.c

In HTTP_new_bio(), if the host has a trailing '/' we took a copy of the hostname but failed to terminate it properly.

2. If an empty password is supplied still try to use it

If an empty password was supplied we ignored it and were trying to use the fallback method to read the password instead (i.e. read from stdin). However if that failed (which it always does if the cmp option -batch is used) then we were reporting that we had successfully read the password without actually setting one.

Instead, if an empty password is explicitly provided we should use it. If no password is supplied explicitly and we have no fallback method then we assume the empty password.

[extended tests]

[mattcaswell]

@DDvO

[DDvO]

Thanks for the fixes; LGTM.

I mentioned a couple of potential coding style enhancements, but these are optional and unrelated to the given fixes.

[DDvO]

Extended test still fail for the Travis CI build witth configuration
no-asm enable-ubsan enable-rc5 enable-md2 enable-ssl3 enable-ssl3-method enable-nextprotoneg no-shared enable-buildtest-c++ -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION -Wno-unused-command-line-argument.

It turns out that this is (as in #12145) due to some CMP verification steps being omitted to allow for more aggressive fuzzing in case FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION is enabled.
In such cases test_cmp_cli does not really make sense.

I had already tried disabling such tests in test/recipes/81-test_cmp_cli.t in #12175 as follows:

plan skip_all => "These tests are not supported in a fuzz build"
    if !disabled("fuzz-libfuzzer") || !disabled("fuzz-afl");

but this turns out to be insufficient here.

Can we somehow check using Perl in that script if the configuration has been done with
-D FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION?
The above condition !disabled("fuzz-libfuzzer") || !disabled("fuzz-afl") should be replaced by that new check.

[DDvO]

This PR is currently blocked by the following question:

Can we somehow check using Perl in that script if the configuration has been done with
-D FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION?

[richsalz]

Not sure the best way to do it, but this probably works:

use lib bldtop_dir('.');
import configdata;
if ( $config{options} =~ /-DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION/ ) {
 ...
}

[DDvO]

Not sure the best way to do it, but this probably works:

use lib bldtop_dir('.');
import configdata;
if ( $config{options} =~ /-DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION/ ) {
 ...
}

Thanks @richsalz - the idea is good,
I just had to adapt this because bldtop_dir is not yet available when importing libs.

@mattcaswell, if you add to test/recipes/81-test_cmp_cli.t the import of bldtop_file and replace

plan skip_all => "These tests are not supported in a fuzz build"
    if !disabled("fuzz-libfuzzer") || !disabled("fuzz-afl");

by

open(CONFIG, bldtop_file("configdata.pm"));
plan skip_all => "These tests are not supported in a fuzz build"
    if (grep { /-DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION/ } <CONFIG>);
close CONFIG;

the exclusion of fuzz-related builds should work fine.

[mattcaswell]

Updated with a new commit that should address the travis failures. I was able to tweak @richsalz's variant a bit more to get it to work.

[mattcaswell]

I just realised I forgot to address @DDvO's style comments. Fix up pushed addressing those.

[DDvO]

@mattcaswell, this is ready to merge. Shall I do?

[mattcaswell]

@mattcaswell, this is ready to merge. Shall I do?

It's not been re-approved since the latest changes - so unfortunately not yet ready for merge.

[DDvO]

Oops, overlooked the need for renewed approval.
Still LGTM.

[t8m]

Unfortunately it is not clear whether all the extended tests are fixed because the fixup commit makes them not to run.

[mattcaswell]

Darn...I remembered to add "extended tests" to my earlier fixup - but not to the subsequent one. Will add shortly.

[t8m]

I've restarted the Travis build on the previous commit to find out as the fixup commit should not change anything in regards to tests.

[mattcaswell]

I've restarted the Travis build on the previous commit to find out as the fixup commit should not change anything in regards to tests

Ah! Ok - perfect. Thanks.

[mattcaswell]

I've restarted the Travis build on the previous commit to find out as the fixup commit should not change anything in regards to tests.

The Travis run passed.

[mattcaswell]

Pushed. Thanks.