On 8/2/2022 5:32 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>
>> +static int copy_uri_to_file(const char *filename, const char *uri)
>> +{
>> + const char *out;
>> +
>> + if (istarts_with(uri, "https:") ||
>> + istarts_with(uri, "http:"))
>
> Let's be a bit more strict to avoid mistakes and make the code
> immediately obvious, e.g.
>
> if (istarts_with(uri, "https://";) ||
> istarts_with(uri, "http://";))
>
>> + return download_https_uri_to_file(filename, uri);
>> +
>> + if (!skip_prefix(uri, "file://", &out))
>> + out = uri;
>
> If we are using istarts_with because URI scheme name is case
> insensitive, shouldn't we do the same for "file://" URL, not
> just for "http(s)://" URL? IOW
>
> if (!skip_iprefix(uri, "file://", &out))
Good ideas. Of course, we don't have a skip_iprefix(), but
I can use "istarts_with()" and then manually add the length.
If we see more need for that in the future, we can consider
adding it.
(It's interesting that these uses in bundle-uri.c are the
only uses of istarts_with() that I see in the codebase.)
>> +static int download_https_uri_to_file(const char *file, const char *uri)
>> {
>> + int result = 0;
>> + struct child_process cp = CHILD_PROCESS_INIT;
>> + FILE *child_in = NULL, *child_out = NULL;
>> + struct strbuf line = STRBUF_INIT;
>> + int found_get = 0;
>> +
>> + strvec_pushl(&cp.args, "git-remote-https", "origin", uri, NULL);
>
> Does "git-remote-https" talk to a "http://"; URL just fine when uri
> parameter starts with "http://";? Would it be the same if the uri
> parameter begins with say "Http://"?
I did a quick check of our HTTPS tests modifying the HTTPD_PROTO
variable in lib-httpd.sh to "HtTP" and we get this fun error:
+ git clone --filter=blob:limit=0 HtTP://127.0.0.1:5601/smart/server client
Cloning into 'client'...
git: 'remote-HtTP' is not a git command. See 'git --help'.
So I guess I can keep case-sensitive comparisons here.
Thanks,
-Stolee
