Re: [BUG?] google code http auth weirdness

On Sat, Mar 16, 2013 at 06:11:32PM -0700, Shawn O. Pearce wrote:

quoted

That seems kind of crazy to me. It's generating an HTTP 200 just to tell
us the credentials are wrong. Which kind of makes sense; it's the only
way to convince the git client to show a custom message when it aborts
(rather than producing its own client-side "authorization failed"
message).

Actually the reason here wasn't to use a custom message. It was to
avoid the client from entering into the old /info/refs fallback path
that existed between 703e6e76 (Git 1.6.6.2) and 6ac964a62 (Git
1.7.12.3). During these versions non-200 responses from the smart
request usually led to a useless error in the client. I suggested to
the Google Code team when they implemented Git support to use a 200
response with the ERR header to give the end-user something easier to
understand than what the Git client have otherwise printed.

OK, that makes sense. I do wonder, though, if the loss of the custom
message will be bad for Google Code. The current message is:

  fatal: remote error: Invalid username/password.
  You may need to use your generated googlecode.com password; see
  https://code.google.com/hosting/settings

which is an important clue (this being the first time I had used Google
Code to authenticate, I was quite surprised that the password was
different from the web page's password, and that pointer helped me
figure it out more quickly).

quoted

  1. Always return a 401 for bad credentials. This means it would lose
     the custom message. But I think that is a good indication that the
     client should be putting more effort into showing the body of the
     401. Probably not all the time, as we do not want to spew HTML at
     the user, but we could perhaps relay error bodies if the
     content-type is "application/x-git-error" or something.

     The downside is that even if we make such a client change and the
     the Google Code server change sit's behavior, people on older git
     clients will lose the benefit of the message.

I don't think this is the way to handle errors in the protocol. I
prefer the existing approach of sending a 200 OK with the ERR header
to indicate the message to show to the client. It works since 1.6.6
when smart HTTP was introduced, and it has a very specific meaning.
The 200 is stating the transport worked, and the ERR is stating the
Git operation did not. :-)

Sorry, I should have been more clear. I mean only to use it for
transport errors, not git errors. So we should not in general be
converting 200 responses into something else (with the exception of the
401, because it does have a transport meaning). But when we _do_ issue
an unsuccessful HTTP code, we should include a message that can be
relayed to the user.

Where we have an issue is on a recoverable authentication error.
Recoverable authentication errors should use 401 so the client can try
again. I don't know how older (e.g. 1.6.6) clients behave here with a
401 response. I guess I should crack out a 1.6.6 build and test.

Before 42653c0 (Prompt for a username when an HTTP request 401s,
2010-04-01), in v1.7.1.1, I think you just get something like:

  error: 401 Authorization failed
  fatal: HTTP request failed

I don't work on Google Code, but I have asked the team to consider
making at least this change.

Thanks for looking into it.

-Peff