On Sun, May 03, 2020 at 02:58:26AM -0400, Jeff King wrote: > On Sat, May 02, 2020 at 11:34:23PM -0700, Carlo Marcelo Arenas Belón wrote: > > > the order of parameters used in credential_match was inconsistent > > between credential.c and credential.h as well, so update both to > > match the current implementation. > > Yes, looks like this has been wrong since the beginning in 118250728e > (credential: apply helper config, 2011-12-10). I checked the callers to > make sure none of them had gotten it backwards, but they all look right > (and just the declaration is wrong). thanks for checking, will include this (and your typo fix) in the submission; should I add your "Reviewed-by" then? was also curious about the other documentation updates mentioned[1] by Jonathan, and that I was hoping this patch will be included with. some things that I think might need clarification (or maybe even code changes after agreed on) are : * the meaning of "exactly" for matching protocol and hostname in the URL since 06 are both case insensitive per RFC3986 and we have been ambiguous on that, leading to some helpers assuming case or encoding. * the rules for how helper matching are expected to be ordered, specially considering the recent adding of wildcard matching and the revival of partial matching, and the fact that the order is relevant for both discovery of credentials and which helpers are used. * the use of hostname as a key, since the addition of cert:// that has none and uses path instead (had emailed the author privately for clarification, but hadn't heard yet) and the effect that has on which fields are expected and which values are valid. * the role of overrides (ex: the documented example of passing URL and later updating it seems useful, eventhough I am not aware if being used) * clarification on which fields can be updated by the helper; currently I don't think we allow overrides for protocol and host and assume all others but the documentation doesn't clarify, and that might be a problem for cert:// where file is more relevant. Carlo [1] https://lore.kernel.org/git/20200428055514.GB201501@xxxxxxxxxx/
