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

Match Dev/Prod parity for Index Page #11514

Merged
merged 1 commit into from Jul 22, 2013

Conversation

schneems
Copy link
Member

With Rails 4 the default index page was moved from a static file index.html inside the public/ folder to an internal controller/view inside of the railties gem. This was to allow use of erb in the default index page and to remove the requirement that new apps must delete a static file to make their index pages work. While this was a good change, the functionality was unexpected to developers who wish to get their apps running in production ASAP. They will create a new app rails new my app, start a server to verify it works, then immediately deploy the app to verify that it can start working in production. Unfortunately locally they see a page when they visit localhost:3000 when they visit their production app they get an error page.

We initially anticipated this problem in the original pull request, but did not properly anticipate the severity or quantity of people who would like this functionality. Having a default index page serves as an excellent litmus test for a passed deploy on default apps, and it is very unexpected to have a page work locally, but not on production.

This change makes the default index page available in production if the developer has not over-written it by defining their own root path inside of routes.

@mdespuits
Copy link

👍

@egilburg
Copy link
Contributor

Why not generate this route in users' routes.rb file? It makes it more obvious for users that this route exists with a new rails app, and makes it easier to alter or remove if the user so desires.

@prathamesh-sonpatki
Copy link
Member

@scneems https://github.com/rails/rails/blob/master/railties/lib/rails/templates/rails/welcome/index.html.erb#L230
This line should be changed also. Because now it is in all environments. Not only in development mode.

With Rails 4 the default index page was moved from a static file `index.html` inside the `public/` folder to an internal controller/view inside of the railties gem. This was to allow use of erb in the default index page and to remove the requirement that new apps must delete a static file to make their index pages work. While this was a good change, the functionality was unexpected to developers who wish to get their apps running in production ASAP. They will create a new app `rails new my app`, start a server to verify it works, then immediately deploy the app to verify that it can start working in production. Unfortunately locally they see a page when they visit `localhost:3000` when they visit their production app they get an error page.

We initially anticipated this problem in the original pull request, but did not properly anticipate the severity or quantity of people who would like this functionality. Having a default index page serves as an excellent litmus test for a passed deploy on default apps, and it is very unexpected to have a page work locally, but not on production. 

This change makes the default index page available in production if the developer has not over-written it by defining their own `root` path inside of routes.
@schneems
Copy link
Member Author

@prathamesh-sonpatki fixed the copy on that page. Good catch.

@prathamesh-sonpatki
Copy link
Member

😄

@gaurish
Copy link
Contributor

gaurish commented Jul 21, 2013

Question:
What if the WelcomeController stops being an internal thing & we make it a normal controller which resides in app/controllers dir, along with ApplicationController, so its completely transparent to the user.

In other words,
What advantage do we gain out of keeping the controller inside railties. is it just the convenience of not having to delete/rename controller or are there other reasons to keep the index page private?

@schneems
Copy link
Member Author

That was the original idea, it was shot down in favor of an internal
controller/route combo.

On Sunday, July 21, 2013, Gaurish Sharma wrote:

Question:
What if the WelcomeController stops being an internal thing & we make it
a normal controller which resides in app/controllers dir, along with
ApplicationController, so its completely transparent to the user.

In other words,
What advantage do we gain out of keeping the controller inside railties.
is it just the convenience of not having to delete/rename controller or are
there other reasons to keep the index page private?


Reply to this email directly or view it on GitHubhttps://github.com//pull/11514#issuecomment-21315814
.

@gaurish
Copy link
Contributor

gaurish commented Jul 21, 2013

Read the entire discussion. so it seems the main point of keeping this internal is skip to generate bunch of files which only have to be deleted; everytime you start an new rails project which makes sense. However, since its the proficient rubyists we are talking about -- I personally think their needs would have been better served by a custom application template but since that PR(#8468) has already been merged & shipped. lets accept that this is the direction we would be going -- internal controllers.

Coming back to topic,
I was hit by this Dev Vs Prod mismatch today(#11502) & ended up wasting an hour debugging. if Development & Pro Env are identical, such silly issues can be prevented in future. Hence, I too was thinking submitting a PR on this. you happen to beat me to it.

Thank you for doing the work, so I didn't have to 👻 Just kidding, hope you won't mind 😉

@drogus
Copy link
Member

drogus commented Jul 21, 2013

@schneems could you add a test?

@robin850
Copy link
Member

@drogus : Still the case here no?

@drogus
Copy link
Member

drogus commented Jul 22, 2013

@robin850 sorry, I totally missed that it's a test for production

drogus added a commit that referenced this pull request Jul 22, 2013
@drogus drogus merged commit 230d2f8 into rails:master Jul 22, 2013
@robin850
Copy link
Member

@drogus : No problem! Glad to help. By the way, thanks for merging! :-)

@rafaelfranca
Copy link
Member

I'm 👎 about this. If we are including the info page on production we are exposing information that should not be exposed.

@rafaelfranca
Copy link
Member

Also, due some mistake, if someone remove the root route in a production environment all users will see this page leaking a lot of important information like Rails version and Ruby patch level. Without this code the user will see the a 404 page, what is expected.

@carlosantoniodasilva
Copy link
Member

I don't feel like exposing the welcome page on production (even though it could be overridden by a new root page added by the developer) is a really good thing.

@lucasmazza
Copy link
Contributor

Having a default index page serves as an excellent litmus test for a passed deploy on default apps, and it is very unexpected to have a page work locally, but not on production.

Altough I never saw such usage of Rails index page I get why some people might try something like that, so maybe to address this Rails should render a minimal template for production env, instead of exposing that amount of information. Maybe just the "Welcome aboard, You’re riding Ruby on Rails!" + the Rails logo should be enough for this.

@josevalim
Copy link
Contributor

Agreed, showing the full index page in production is not a good idea. Also keep in mind that some apps (APIs) may not even have an index page.

@gaurish
Copy link
Contributor

gaurish commented Jul 22, 2013

Which critical information will get exposed?

 if Rails.env.development?
          app.routes.append do
            get '/rails/info/properties' => "rails/info#properties"
            get '/rails/info/routes' => "rails/info#routes"
            get '/rails/info' => "rails/info#index"
          end
        end

as you may notice, the critical information such ruby version etc will only
be available in /rails/info/ routes which are only defined in
development.

this static page exists in all rails version < 4. And gets displayed both production & development Envs even today.

@josevalim
Copy link
Contributor

Another solution to this problem is to add a note to the regular index on development showing it is only available in development.

@rafaelfranca
Copy link
Member

I prefer to keep this page only on development, with 404 on production.

@gaurish
Copy link
Contributor

gaurish commented Jul 22, 2013

Another solution to this problem is to add a note to the regular index on development showing it is only available in development.

Yes, we can add a simple conditional, unless Rails.env.development & then only render this line. Else it would a normal static page with no info

@rafaelfranca
Copy link
Member

I still don't see any strong reason to render this page on production.

@steveklabnik
Copy link
Member

Yup, I tend to lean towards 404 in production as well.

@drogus
Copy link
Member

drogus commented Jul 22, 2013

I agree on the argument that not removing this page on production may expose information, but then I really think that this is a step backwards from having index.html.

I will revert this thing, but please think what are we actually gaining by the original page - a bit more info on the index page in exchange for a lot of confusion. If you still think that this is a good direction, I won't argue, but please think about it for a moment.

@drogus
Copy link
Member

drogus commented Jul 22, 2013

For a reference, reverted with 73bbf54

@schneems
Copy link
Member Author

On security: right now serving this page in production gives away that we are running rails (though default error pages also do) and it may hint as to the version of rails since you could do a diff of the page versus known default rails index pages. Any of the app specific info is behind other urls that are not available in production.

The original intent of moving away from static index.html is to avoid confusion for new comers who will add a root but don't know you need to also delete a file to make that work. And for seasoned devs it removes one step from a "typical" rails app.

Unfortunately it took away the quick rails new ; deploy check some people and quite a few tutorials are using.

An optimal solution will minimize confusion around deleting any pages, provide getting started information for new developers on first rails new, and minimize differences between dev and prod behavior. All of this without exposing any sensitive data. This PR was my attempt at addressing all of those points, I'm still open to alternative implementations. What are some other options for achieving all of these goals?

@rafaelfranca
Copy link
Member

There are a some of differences between dev and prod behavior. For example, assets are not precompiled by default on production and production environment use eager loading of constants.

I don't see any problem to have these differences, and the index page returning 404 is fine to me too.

Can not these tutorials being updated to actually tell the people what is going on with the information page? Doing this I think will be more useful since can be a link to tell for the users that Rails, by convention, have diferrent behaviors depending on the environment.

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