Derrick Stolee <derrickstolee@xxxxxxxxxx> writes: >>> + 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://";)) This part (i.e. check for "<scheme>://", not "<scheme>:") still stands, as the latter could be an scp style Git URL that goes to the host whose name is <scheme>. As mentioned later, s/istarts/starts/ is probably a good thing to do here. >> 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. Guarding them to lowercase-only may sound like a cop-out to purists, but I think it is reasonable thing to do. The only folks that would be offended by are protocol lawyers, and as your check shows, we are treating <scheme> case sensitively already. An obvious alternative is to downcase the "<scheme>://" part but I do not think it is worth it; we have to do that everywhere and we need to be confident that we covered all the code paths---the latter is expensive. There do exist skip_iprefix() that are used in many places, like convert, http, mailinfo, etc. by the way. That is a moot point as we are not doing case insensitive comparison anymore. Thanks.
