In PR git-lfs#4781 we updated a number of packages to replace global string
variables with local variables so they could be translated via the
"tr" package's localization functions.

One concern raised in that PR pertained to the changes to the
lfshttp/errors.go source file, where a map of error format strings
is constructed in the defaultError() function, with each string
mapped to a specific HTTP status code.  These strings should contain
at least one "%s" verb, into which the HTTP request URL should be
interpolated.

In commit cda9c3c of PR git-lfs#4781 we moved
the map's initialization into the defaultError() function of our
"lfshttp" package, and added a call to the Get() method of the Tr
structure from our "tr" package around each string's definition.
However, we deferred the interpolation of the request URL to a
subsequent call to a string formatting function, with the intent that
we would first select the correct localized format string from the
map based on the HTTP request's status code, and then interpolate
the request's URL into the final error message.

Further, if a request's status code does not correspond to a string
in the map, our defaultError() function creates a generic error
message into which we try to interpolate both the request's URL and
its status code.  For these generic messages, the intent of the
changes in commit cda9c3c was that
we first interpolate the status code, and then treat the resultant
string as if it had been found in the defaultErrors map, meaning it
should still contain one "%s" verb into which the request URL can be
interpolated.

Such a design implies that in the generic error messages we need to
use a "%d" verb for the status code, and a doubly-escaped "%%s" verb
for the request URL, since we want the latter to be converted to a
regular "%s" verb when the string is first used as a format string.

This design exposed an unexpected issue, which is described in
leonelquinteros/gotext#57.  In trying to work around this issue, though,
we introduced a bug of our own whereby the generic messages are passed
through three formatting functions rather than two, and in doing so
the "%%s" verbs are eliminated before we attempt to interpolate the
HTTP request's URL, leading to further errors.

The single commit in PR git-lfs#5822 attempts to fix this problem, but without
complete success.  We now adopt the changes from that PR's commit and
resolve the remaining problems so we should always supply an appropriate
set of arguments for the number of formatting verbs in both types of
messages in the defaultError() function.

The confusion here stems from the issue reported in
leonelquinteros/gotext#57, namely that the Printf() function of the
"github.com/leonelquinteros/gotext" package behaves differently
depending on whether or not it is passed any arguments other than
a format string.  When no arguments are passed, the format string
is not parsed and any verbs are left unaltered, whereas when
at least one additional argument is passed, the format string is
parsed and the trailing arguments interpolated into it.

The Tr global variable in our "tr" package is a Locale structure
from the "gotext" package.  Starting with commit
cda9c3c, we separately invoke this
variable's Get() method on each of the error messages defined in
the defaultErrors map, but without any other arguments.  Because
the Get() method does not parse the format string when no other
arguments are passed, in theory we ought to be able to define these
messages with regular "%s" verbs.  If we do so, though, the Go linter
complains that it appears we are passing format strings to the Get()
method without matching arguments for their "%s" verbs.

To work around this linter limitation, we currently use "%%s" verbs in
the error messages in the defaultErrors map, which are then passed
through the Get() method unchanged.  We then make a subsequent call
to the Sprintf() function from the Go standard library's "fmt" package,
passing the error message as the format string but without any other
arguments.  This call effectively just converts any "%%s" verbs to
"%s" verbs.

By contrast, the generic error messages in the defaultError() function
are passed to the Tr variable's Get() method with another argument,
the HTTP request's status code.  Due to the presence of the extra
argument, the Get() method parses the format string, replacing the "%d"
verb with the status code and converting the "%%s" verb into a "%s"
verb.  (Note that originally, in PR git-lfs#4781, we used the Sprintf()
function of the standard "fmt" package rather than the Get() method
of our Tr variable.  We corrected this oversight in commit
04abbd8 of PR git-lfs#4846, but while that
change allows the strings to be localized, it has no effect on the
handling of the formatting verbs.)

As with the error messages from the defaultErrors map, the generic
error messages are then also passed without any other arguments to the
Sprintf() function from the standard "fmt" package.  However, this call
does not have the same effect in both cases.  When the error message is
specific to an HTTP status code and has been selected from the
defaultErrors map, this call just converts the "%%s" verb into a "%s"
one, as noted above.  When the error message is a generic one, though,
that conversion has already been performed by the Tr structure's Get()
method.  The consequence is that the Sprintf() function replaces the
"%s" verb with "%!s(MISSING)", as described in the "fmt" package's
documentation:

  https://pkg.go.dev/fmt#hdr-Format_errors

In the final step of the defaultError() function we then pass the
result from this Sprintf() call to the Errof() function of our
"errors" package, and here we include the HTTP request's URL as a second
argument.  In the case where the error message has been selected from
the defaultErrors map, this last formatting function replaces the "%s"
verb with the URL.  In the case where the error message is a generic one,
though, the leading portion of the format error marker "%!s(MISSING)"
cannot be parsed as a verb, and so the "%!" is replaced with a full
reflection of all the internal elements of the URL structure into
string form.

Since these format error markers and reflection fields are not desirable
in our HTTP request error messages, we revise the defaultError() function
so it each type of error message is passed through only a single
formatting function, specifically the Tr variable's Get() method.
This allows us to use regular "%s" and "%d" formatting verbs exclusively
in all the messages.

Rather than call the Get() method on each string as it is defined in
the defaultErrors map, we only call the method once a specific message
is selected based on the HTTP request's status code, and we pass the
request's URL to the Get() method as another argument.  For the generic
error messages, we simply pass both the status code and the URL as
arguments to the Get() function.  Because we always pass either one or
two arguments to that function in addition to a format string, it always
performs the appropriate interpolation for us, and we avoid the need
to work around its behaviour when it is only passed a format string
without other arguments, as well as avoiding the use of any further
formatting functions like Sprintf().

Co-authored-by: Philip Peterson <[email protected]>

In PR git-lfs#4781 we updated a number of packages to replace global string
variables with local variables so they could be translated via the
"tr" package's localization functions.

One concern raised in that PR pertained to the changes to the
lfshttp/errors.go source file, where a map of error format strings
is constructed in the defaultError() function, with each string
mapped to a specific HTTP status code.  These strings should contain
at least one "%s" verb, into which the HTTP request URL should be
interpolated.

In commit cda9c3c of PR git-lfs#4781 we moved
the map's initialization into the defaultError() function of our
"lfshttp" package, and added a call to the Get() method of the Tr
structure from our "tr" package around each string's definition.
However, we deferred the interpolation of the request URL to a
subsequent call to a string formatting function, with the intent that
we would first select the correct localized format string from the
map based on the HTTP request's status code, and then interpolate
the request's URL into the final error message.

Further, if a request's status code does not correspond to a string
in the map, our defaultError() function creates a generic error
message into which we try to interpolate both the request's URL and
its status code.  For these generic messages, the intent of the
changes in commit cda9c3c was that
we first interpolate the status code, and then treat the resultant
string as if it had been found in the defaultErrors map, meaning it
should still contain one "%s" verb into which the request URL can be
interpolated.

Such a design implies that in the generic error messages we need to
use a "%d" verb for the status code, and a doubly-escaped "%%s" verb
for the request URL, since we want the latter to be converted to a
regular "%s" verb when the string is first used as a format string.

This design exposed an unexpected issue, which is described in
leonelquinteros/gotext#57.  In trying to work around this issue, though,
we introduced a bug of our own whereby the generic messages are passed
through three formatting functions rather than two, and in doing so
the "%%s" verbs are eliminated before we attempt to interpolate the
HTTP request's URL, leading to further errors.

The single commit in PR git-lfs#5822 attempts to fix this problem, but without
complete success.  We now adopt the changes from that PR's commit and
resolve the remaining problems so we should always supply an appropriate
set of arguments for the number of formatting verbs in both types of
messages in the defaultError() function.

The confusion here stems from the issue reported in
leonelquinteros/gotext#57, namely that the Printf() function of the
"github.com/leonelquinteros/gotext" package behaves differently
depending on whether or not it is passed any arguments other than
a format string.  When no arguments are passed, the format string
is not parsed and any verbs are left unaltered, whereas when
at least one additional argument is passed, the format string is
parsed and the trailing arguments interpolated into it.

The Tr global variable in our "tr" package is a Locale structure
from the "gotext" package.  Starting with commit
cda9c3c, we separately invoke this
variable's Get() method on each of the error messages defined in
the defaultErrors map, but without any other arguments.  Because
the Get() method does not parse the format string when no other
arguments are passed, in theory we ought to be able to define these
messages with regular "%s" verbs.  If we do so, though, the Go linter
complains that it appears we are passing format strings to the Get()
method without matching arguments for their "%s" verbs.

To work around this linter limitation, we currently use "%%s" verbs in
the error messages in the defaultErrors map, which are then passed
through the Get() method unchanged.  We then make a subsequent call
to the Sprintf() function from the Go standard library's "fmt" package,
passing the error message as the format string but without any other
arguments.  This call effectively just converts any "%%s" verbs to
"%s" verbs.

By contrast, the generic error messages in the defaultError() function
are passed to the Tr variable's Get() method with another argument,
the HTTP request's status code.  Due to the presence of the extra
argument, the Get() method parses the format string, replacing the "%d"
verb with the status code and converting the "%%s" verb into a "%s"
verb.  (Note that originally, in PR git-lfs#4781, we used the Sprintf()
function of the standard "fmt" package rather than the Get() method
of our Tr variable.  We corrected this oversight in commit
04abbd8 of PR git-lfs#4846, but while that
change allows the strings to be localized, it has no effect on the
handling of the formatting verbs.)

As with the error messages from the defaultErrors map, the generic
error messages are then also passed without any other arguments to the
Sprintf() function from the standard "fmt" package.  However, this call
does not have the same effect in both cases.  When the error message is
specific to an HTTP status code and has been selected from the
defaultErrors map, this call just converts the "%%s" verb into a "%s"
one, as noted above.  When the error message is a generic one, though,
that conversion has already been performed by the Tr structure's Get()
method.  The consequence is that the Sprintf() function replaces the
"%s" verb with "%!s(MISSING)", as described in the "fmt" package's
documentation:

  https://pkg.go.dev/fmt#hdr-Format_errors

In the final step of the defaultError() function we then pass the
result from this Sprintf() call to the Errof() function of our
"errors" package, and here we include the HTTP request's URL as a second
argument.  In the case where the error message has been selected from
the defaultErrors map, this last formatting function replaces the "%s"
verb with the URL.  In the case where the error message is a generic one,
though, the leading portion of the format error marker "%!s(MISSING)"
cannot be parsed as a verb, and so the "%!" is replaced with a full
reflection of all the internal elements of the URL structure into
string form.

Since these format error markers and reflection fields are not desirable
in our HTTP request error messages, we revise the defaultError() function
so it each type of error message is passed through only a single
formatting function, specifically the Tr variable's Get() method.
This allows us to use regular "%s" and "%d" formatting verbs exclusively
in all the messages.

Rather than call the Get() method on each string as it is defined in
the defaultErrors map, we only call the method once a specific message
is selected based on the HTTP request's status code, and we pass the
request's URL to the Get() method as another argument.  For the generic
error messages, we simply pass both the status code and the URL as
arguments to the Get() function.  Because we always pass either one or
two arguments to that function in addition to a format string, it always
performs the appropriate interpolation for us, and we avoid the need
to work around its behaviour when it is only passed a format string
without other arguments, as well as avoiding the use of any further
formatting functions like Sprintf().

Co-authored-by: Philip Peterson <[email protected]>