rc.* helpers are named incorrectly

Issue #81 open
Jacob Kaplan-Moss
created an issue

RFC 2616 defines names for the various HTTP response codes. Unfortunately, the names Piston uses for the rc.* helpers don't match. I found this really confusing, and it led to a number of places where I was using HTTP codes incorrectly.

These helpers should be renamed to match the RFC (http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html). These differences are important. Here's the differences between Piston and RFC; I'll explain why each is a problem below:

{{{ Code Piston's name RFC Note 200 OK OK
201 CREATED Created
204 DELETED No Content [1] 400 BAD_REQUEST Bad Request
401 FORBIDDEN Unauthorized [2] 404 NOT_FOUND Not Found
409 DUPLICATE_ENTRY Conflict [3] 410 NOT_HERE Gone [4] 501 NOT_IMPLEMENTED Not Implemented 503 THROTTLED Service Unavailable [5] }}}

Okay, so why are these names misleading?

204 is indeed the correct response to a DELETE. However, it's also used in a number of other contexts -- it's a valid response to a PUT that updates a resource, for example. The RFC says that 204 is used whenever "the server has fulfilled the request but does not need to return an entity-body," which is a lot more often than after a DELETE.

This is actually a really big problem. HTTP has two different types of "authentication failure" codes, and this naming confuses the two.

401 means "unauthorized"; it indicates that the given request requires authorization. The response can include a WWW-Authenticate header indicating how authentication ought to be performed. The RFC notes that "the client MAY repeat the request with a suitable Authorization header field". 403, on the other hand, is "forbidden"; it indicates that "authorization will not help and the request SHOULD NOT be repeated." You might return a 403 if the password given for the user doesn't match, for example. Naming 401 "Forbidden" is confusing, and wrong. It was that discovery that prompted me to write this bug.

409 conflict usually does indicate a duplicate entry, but it's also often used in transactional contexts to indicate that someone else's transaction conflicted with yours and prevented commit. The naming obscures that (and other) uses.

"Not here" sounds a lot like "Not found" to me. "Gone", on the other hand, indicates that the resource once was there, but is no longer. There's an important difference which (again) the name obscures.

503 is useful for a lot more than throttling.

Again, please rename these codes to be consistent with the RFC.

Comments (8)

  1. Simon Law

    I agree with jacobian. Included is a patch that clarifies these problems and adds a few new status codes.

    As well, the default update() method returns the updated instance, to confirm that the resource you created was saved properly. And the default delete() method returns rc.NOT_FOUND as opposed to rc.GONE, since GONE means that the resource will never reappear.

  2. Simon Law

    Hmm, perhaps you could provide duplicates for the old names as compatibility. But rc.FORBIDDEN is really confusing and we discovered that we were using it incorrectly by accident.

    Is there some setting that could be provided to put the rc_factory into compatibility mode? And then some documentation on upgrade?

  3. Jesper Noehr repo owner

    I've put a lot of effort into keeping backwards compatibility on all changes, with the exception of using warnings.

    It's difficult to distinguish between correct and incorrect usage of the ones that merely changed though, so what I suggest is, we keep the removed ones around for legacy purposes, and update the documentation. That shouldn't break too many things.

  4. Simon Law

    Well, rc.FORBIDDEN is the one that changed that's actually important. To me (and probably jacobian) this is a bug fix and probably requires authors to grep through their code to check if they really meant 401 or 403 for every use of rc.FORBIDDEN.

    Do you agree?

  5. Jesper Noehr repo owner


    I think changing the meaning of a code is "OK", as long as we don't remove ones that people may use, which would consequently result in AttributeError's and code failure.

    We'll document the changed ones on the wiki and let people know via release notes and the mailing list.

  6. Simon Law

    Perfect. Would you like me to submit a new patch with the old values put back? Or will you do that?

    Also, what do you think about using warnings .warn() and DeprecationWarning for use of the old symbols?

  7. Log in to comment