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

Rack changed their status codes to IETF RFC 7231 #20757

Merged
merged 2 commits into from
Aug 22, 2015

Conversation

bquorning
Copy link
Contributor

Rails is calling the Rack::Utils.status_code method in several places, and calling that method with an unknown symbol will return the integer 500.

The valid symbols are defined by the names in Rack::Utils::HTTP_STATUS_CODES. And they changed in in rack/rack#754, released in Rack v1.6.0 (see also https://github.com/rack/rack/blob/master/HISTORY.md). This PR changes Rails’ documentation to match the new valid status symbols.

Here’s the relevant diff from that Rack PR:

     HTTP_STATUS_CODES = {
       100 => 'Continue',
       101 => 'Switching Protocols',
 @@ -590,7 +590,6 @@ def to_params_hash
       303 => 'See Other',
       304 => 'Not Modified',
       305 => 'Use Proxy',
-      306 => 'Reserved',
       307 => 'Temporary Redirect',
       308 => 'Permanent Redirect',
       400 => 'Bad Request',
 @@ -606,12 +605,11 @@ def to_params_hash
       410 => 'Gone',
       411 => 'Length Required',
       412 => 'Precondition Failed',
-      413 => 'Request Entity Too Large',
-      414 => 'Request-URI Too Long',
+      413 => 'Payload Too Large',
+      414 => 'URI Too Long',
       415 => 'Unsupported Media Type',
-      416 => 'Requested Range Not Satisfiable',
+      416 => 'Range Not Satisfiable',
       417 => 'Expectation Failed',
-      418 => 'I\'m a teapot',
       422 => 'Unprocessable Entity',
       423 => 'Locked',
       424 => 'Failed Dependency',
 @@ -625,7 +623,7 @@ def to_params_hash
       503 => 'Service Unavailable',
       504 => 'Gateway Timeout',
       505 => 'HTTP Version Not Supported',
-      506 => 'Variant Also Negotiates (Experimental)',
+      506 => 'Variant Also Negotiates',
       507 => 'Insufficient Storage',
       508 => 'Loop Detected',
       510 => 'Not Extended',

@claudiob
Copy link
Member

claudiob commented Jul 2, 2015

🍵 I'm gonna miss you 😭 😭 😭 😭

Even though the actual change did not happen in the Rails codebase, it might still break someone's app when updating to Rails 5.

I think it might be useful to add a note to the "Upgrade to Rails 5" guide too.

Also, it looks like the failing test is not caused by the commit.

@bquorning
Copy link
Contributor Author

bquorning commented Jul 2, 2015 via email

@bquorning bquorning force-pushed the http-status-codes-changed-in-rack branch from a7134f5 to 3e6e6b9 Compare July 3, 2015 07:35
@bquorning
Copy link
Contributor Author

Added a commit with upgraded 4.2 release notes.

@ncalca
Copy link

ncalca commented Jul 3, 2015

👍

1 similar comment
@dasch
Copy link
Contributor

dasch commented Jul 3, 2015

👍

@rails rails locked and limited conversation to collaborators Jul 3, 2015
@rails rails unlocked this conversation Jul 26, 2015
@bquorning
Copy link
Contributor Author

Ping @spastorino who merged rack/rack#754 into Rack master. Sorry if pinging is considered rude 😊

@@ -227,6 +227,17 @@ restore the old behavior.
If you do this, be sure to configure your firewall properly such that only
trusted machines on your network can access your development server.

### Changed status option symbols for `render`

Due to a [change in Rack](https://github.com/tonyta/rack/commit/be28c6a2ac152fe4adfbef71f3db9f4200df89e8), the symbols that the `render` method accepts for the `:status` option have changed:
Copy link
Contributor

Choose a reason for hiding this comment

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

You should point to the rack repository here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops, you’re right.

@spastorino
Copy link
Contributor

I think we should revert this in Rack 1.6.x and release again and apply only to 2.x. For some reason I thought it was only applied to 2.x.

@bquorning
Copy link
Contributor Author

It was released in Rack 1.6.0 more than 8 months ago. Isn’t it a bit too late to revert a breaking change?

My guess is that a lot of applications have already been changed to work around this change. It would be confusing if they had to change the behavior back the next time they upgrade Rack.

@bquorning bquorning force-pushed the http-status-codes-changed-in-rack branch from c073a1d to 3ec3ac6 Compare August 21, 2015 07:04
spastorino added a commit that referenced this pull request Aug 22, 2015
…-rack

Rack changed their status codes to IETF RFC 7231
@spastorino spastorino merged commit 4115a12 into rails:master Aug 22, 2015
@bquorning bquorning deleted the http-status-codes-changed-in-rack branch August 22, 2015 20:03
@gamafranco
Copy link

Mind me to update the integration with my PiTeapot...

@BenMorganIO
Copy link
Contributor

Wait, I use I'm a Teapot! :(

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.

None yet

7 participants