On Sat, Mar 16, 2013 at 06:11:32PM -0700, Shawn O. Pearce wrote: > > 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). > > 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html