Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for HTTP 451 "Unavailable for Legal Reasons" #985

Merged
merged 1 commit into from
Dec 22, 2015

Conversation

ktheory
Copy link
Contributor

@ktheory ktheory commented Dec 21, 2015

Adds support for the newly approved HTTP 451 response code,
“Unavailable for Legal Reasons”.

Adds Rack::Response#unavailable? method.

See also:

@@ -128,6 +128,7 @@ def not_found?; status == 404; end
def method_not_allowed?; status == 405; end
def precondition_failed?; status == 412; end
def unprocessable?; status == 422; end
def unavailable?; status == 451; end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be confusing with 503 Service Unavailable. I'd be open to renaming this method to unavailable_for_legal_reasons, but that's awfully wordy. Figured I'd start simple.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have shorthand methods for every status code. Think this is one we'd omit.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Omitting is probably OK. I imagine use of it would be sufficiently rare that the code could be checked directly.

However, for completion, it might be a good idea to keep it. Maybe call it legally_unavailable? or unavailable_legally? or unavailable_by_law? or censored? or censored_by_law?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jeremy, @colindean: 👌 I removed the method in 040de0d.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Please rebase + squash to a single commit as well.

@@ -551,6 +551,7 @@ def names
428 => 'Precondition Required',
429 => 'Too Many Requests',
431 => 'Request Header Fields Too Large',
451 => 'Unavailable For Legal Reasons',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be wise to put a comment on here stating that the code isn't yet present on the IANA list as of the current date, in the unlikely event that someone tries to import from the CSV linked in the comment on line 503?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be wise to put a comment on here

Seems like a stylistic thing. I'm fine either way, and will defer to @jeremy.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be appropriate to mention the exception at #985 (diff) since the comment says exactly how this list is generated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jeremy Ha! Was just writing the comment when I noticed that the IANA CSV was updated to include the new status code. So I updated per the comment on line 503. 🎉

@ktheory ktheory force-pushed the http-451-unavailable-for-legal-reasons branch 2 times, most recently from 65ca111 to c97173f Compare December 22, 2015 01:48
@ktheory
Copy link
Contributor Author

ktheory commented Dec 22, 2015

All squashed & rebased & updated per the IANA CSV of status codes. 😌

Adds support for the newly approved HTTP 451 response code,
“Unavailable for Legal Reasons”.

IETF draft specification:
https://tools.ietf.org/html/draft-tbray-http-legally-restricted-status-0
5

More info:
http://www.451unavailable.org
@ktheory ktheory force-pushed the http-451-unavailable-for-legal-reasons branch from c97173f to 73e0827 Compare December 22, 2015 01:51
jeremy added a commit that referenced this pull request Dec 22, 2015
…easons

Add support for HTTP 451 "Unavailable for Legal Reasons"
@jeremy jeremy merged commit 2859a5b into rack:master Dec 22, 2015
@ktheory ktheory deleted the http-451-unavailable-for-legal-reasons branch December 22, 2015 02:44
@colindean
Copy link

🍻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants