[4/4] credential: document protocol updates
diff mbox series

Message ID 20200505013908.4596-5-carenas@gmail.com
State New
Headers show
Series
  • credential: documentation updates for maint
Related show

Commit Message

Carlo Marcelo Arenas Belón May 5, 2020, 1:39 a.m. UTC
Document protocol changes after CVE-2020-11008, while at it do some
minor improvements for clarity and consitency.

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 Documentation/git-credential.txt | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

Comments

Jeff King May 6, 2020, 4:26 p.m. UTC | #1
On Mon, May 04, 2020 at 06:39:08PM -0700, Carlo Marcelo Arenas Belón wrote:

> Document protocol changes after CVE-2020-11008, while at it do some
> minor improvements for clarity and consitency.

s/consitency/consistency/s

I think the overall direction is good. A few small comments below.

>  `host`::
>  
> -	The remote hostname for a network credential.
> +	The remote hostname for a network credential.  This includes
> +	the port number if one was specified.

It might be worth giving an example of the syntax, like:

  the port number if one was specified (e.g., "example.com:8088").

> @@ -146,8 +147,11 @@ Git understands the following attributes:
>  	value is parsed as a URL and treated as if its constituent parts
>  	were read (e.g., `url=https://example.com` would behave as if
>  	`protocol=https` and `host=example.com` had been provided). This
> -	can help callers avoid parsing URLs themselves.  Note that any
> -	components which are missing from the URL (e.g., there is no
> -	username in the example above) will be set to empty; if you want
> -	to provide a URL and override some attributes, provide the URL
> -	attribute first, followed by any overrides.
> +	can help callers avoid parsing URLs themselves.
> +
> +	Note that specifying a protocol is mandatory and if the URL
> +	type doesn't require a hostname (like for cert://) then an
> +	empty ("")  hostname will be generated.

So we are losing the bit about overriding. I think that is OK, as we'd
like to avoid suggesting that is a good idea, and we may even remove the
feature in the future.

The word "generated" confused me a bit. Maybe something like:

  ...and if the URL doesn't specify a hostname (e.g.,
  "cert:///path/to/file"), the credential will contain a hostname
  attribute whose value is an empty string.

> +	Components which are missing from the URL (e.g., there is no
> +	username in the example above) will be left unset.

Makes sense.

-Peff

Patch
diff mbox series

diff --git a/Documentation/git-credential.txt b/Documentation/git-credential.txt
index 6f0c7ca80f..73a287e634 100644
--- a/Documentation/git-credential.txt
+++ b/Documentation/git-credential.txt
@@ -104,7 +104,7 @@  INPUT/OUTPUT FORMAT
 credential information in its standard input/output. This information
 can correspond either to keys for which `git credential` will obtain
 the login/password information (e.g. host, protocol, path), or to the
-actual credential data to be obtained (login/password).
+actual credential data to be obtained (username/password).
 
 The credential is split into a set of named attributes, with one
 attribute per line. Each attribute is
@@ -123,7 +123,8 @@  Git understands the following attributes:
 
 `host`::
 
-	The remote hostname for a network credential.
+	The remote hostname for a network credential.  This includes
+	the port number if one was specified.
 
 `path`::
 
@@ -134,7 +135,7 @@  Git understands the following attributes:
 `username`::
 
 	The credential's username, if we already have one (e.g., from a
-	URL, from the user, or from a previously run helper).
+	URL, the configuration, the user, or from a previously run helper).
 
 `password`::
 
@@ -146,8 +147,11 @@  Git understands the following attributes:
 	value is parsed as a URL and treated as if its constituent parts
 	were read (e.g., `url=https://example.com` would behave as if
 	`protocol=https` and `host=example.com` had been provided). This
-	can help callers avoid parsing URLs themselves.  Note that any
-	components which are missing from the URL (e.g., there is no
-	username in the example above) will be set to empty; if you want
-	to provide a URL and override some attributes, provide the URL
-	attribute first, followed by any overrides.
+	can help callers avoid parsing URLs themselves.
+
+	Note that specifying a protocol is mandatory and if the URL
+	type doesn't require a hostname (like for cert://) then an
+	empty ("")  hostname will be generated.
+
+	Components which are missing from the URL (e.g., there is no
+	username in the example above) will be left unset.