From e8ee39218695abf79e53e0fb66c0e485ea2b0426 Mon Sep 17 00:00:00 2001
From: Michael Brown <supermathie@gmail.com>
Date: Mon, 15 Jul 2019 17:31:24 -0400
Subject: [PATCH] Revert "FIX: apply defaults constraints to routes format
 (#7890)"

This reverts commit 7d01c5de1a92b020e19f387123a3cbbbe969c727.

Trivial get on / was failing with a 404 with this change.
---
 app/controllers/users_controller.rb    |  5 +-
 config/routes.rb                       | 83 +++++++++++++-------------
 spec/requests/users_controller_spec.rb | 12 ++--
 3 files changed, 47 insertions(+), 53 deletions(-)

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 1fc18795882..bd0bf7ae62d 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -1065,10 +1065,7 @@ class UsersController < ApplicationController
       @confirmed = true
     end
 
-    respond_to do |format|
-      format.json { render json: success_json }
-      format.html { render layout: 'no_ember' }
-    end
+    render layout: 'no_ember'
   end
 
   def list_second_factors
diff --git a/config/routes.rb b/config/routes.rb
index 62e3ae4f397..48f93e5d9a3 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -12,8 +12,7 @@ USERNAME_ROUTE_FORMAT = /[%\w.\-]+?/ unless defined? USERNAME_ROUTE_FORMAT
 BACKUP_ROUTE_FORMAT = /.+\.(sql\.gz|tar\.gz|tgz)/i unless defined? BACKUP_ROUTE_FORMAT
 
 Discourse::Application.routes.draw do
-  scope path: nil, constraints: { format: /(json|html)/ } do
-    relative_url_root = (defined?(Rails.configuration.relative_url_root) && Rails.configuration.relative_url_root) ? Rails.configuration.relative_url_root + '/' : '/'
+  relative_url_root = (defined?(Rails.configuration.relative_url_root) && Rails.configuration.relative_url_root) ? Rails.configuration.relative_url_root + '/' : '/'
 
   match "/404", to: "exceptions#not_found", via: [:get, :post]
   get "/404-body" => "exceptions#not_found_body"
@@ -25,15 +24,13 @@ Discourse::Application.routes.draw do
   post "webhooks/sendgrid" => "webhooks#sendgrid"
   post "webhooks/sparkpost" => "webhooks#sparkpost"
 
-  scope path: nil, constraints: { format: /.*/ } do
-    if Rails.env.development?
-      mount Sidekiq::Web => "/sidekiq"
-      mount Logster::Web => "/logs"
-    else
-      # only allow sidekiq in master site
-      mount Sidekiq::Web => "/sidekiq", constraints: AdminConstraint.new(require_master: true)
-      mount Logster::Web => "/logs", constraints: AdminConstraint.new
-    end
+  if Rails.env.development?
+    mount Sidekiq::Web => "/sidekiq"
+    mount Logster::Web => "/logs"
+  else
+    # only allow sidekiq in master site
+    mount Sidekiq::Web => "/sidekiq", constraints: AdminConstraint.new(require_master: true)
+    mount Logster::Web => "/logs", constraints: AdminConstraint.new
   end
 
   resources :about do
@@ -353,17 +350,17 @@ Discourse::Application.routes.draw do
   post "composer/parse_html" => "composer#parse_html"
 
   resources :static
-  post "login" => "static#enter"
-  get "login" => "static#show", id: "login"
-  get "password-reset" => "static#show", id: "password_reset"
-  get "faq" => "static#show", id: "faq"
-  get "tos" => "static#show", id: "tos", as: 'tos'
-  get "privacy" => "static#show", id: "privacy", as: 'privacy'
-  get "signup" => "static#show", id: "signup"
-  get "login-preferences" => "static#show", id: "login"
+  post "login" => "static#enter", constraints: { format: /(json|html)/ }
+  get "login" => "static#show", id: "login", constraints: { format: /(json|html)/ }
+  get "password-reset" => "static#show", id: "password_reset", constraints: { format: /(json|html)/ }
+  get "faq" => "static#show", id: "faq", constraints: { format: /(json|html)/ }
+  get "tos" => "static#show", id: "tos", as: 'tos', constraints: { format: /(json|html)/ }
+  get "privacy" => "static#show", id: "privacy", as: 'privacy', constraints: { format: /(json|html)/ }
+  get "signup" => "static#show", id: "signup", constraints: { format: /(json|html)/ }
+  get "login-preferences" => "static#show", id: "login", constraints: { format: /(json|html)/ }
 
   %w{guidelines rules conduct}.each do |faq_alias|
-    get faq_alias => "static#show", id: "guidelines", as: faq_alias
+    get faq_alias => "static#show", id: "guidelines", as: faq_alias, constraints: { format: /(json|html)/ }
   end
 
   get "my/*path", to: 'users#my_redirect'
@@ -423,8 +420,8 @@ Discourse::Application.routes.draw do
     get "#{root_path}/:username/messages/group/:group_name/archive" => "user_actions#private_messages", constraints: { username: RouteFormat.username, group_name: RouteFormat.username }
     get "#{root_path}/:username/messages/tags/:tag_id" => "user_actions#private_messages", constraints: StaffConstraint.new
     get "#{root_path}/:username.json" => "users#show", constraints: { username: RouteFormat.username }, defaults: { format: :json }
-    get({ "#{root_path}/:username" => "users#show", constraints: { username: RouteFormat.username } }.merge(index == 1 ? { as: 'user' } : {}))
-    put "#{root_path}/:username" => "users#update", constraints: { username: RouteFormat.username }, defaults: { format: :json }
+    get({ "#{root_path}/:username" => "users#show", constraints: { username: RouteFormat.username, format: /(json|html)/ } }.merge(index == 1 ? { as: 'user' } : {}))
+    put "#{root_path}/:username" => "users#update", constraints: { username: RouteFormat.username, format: /(json|html)/ }, defaults: { format: :json }
     get "#{root_path}/:username/emails" => "users#check_emails", constraints: { username: RouteFormat.username }
     get({ "#{root_path}/:username/preferences" => "users#preferences", constraints: { username: RouteFormat.username } }.merge(index == 1 ? { as: :email_preferences } : {}))
     get "#{root_path}/:username/preferences/email" => "users_email#index", constraints: { username: RouteFormat.username }
@@ -466,7 +463,7 @@ Discourse::Application.routes.draw do
     get "#{root_path}/:username/badges" => "users#badges", constraints: { username: RouteFormat.username }
     get "#{root_path}/:username/notifications" => "users#show", constraints: { username: RouteFormat.username }
     get "#{root_path}/:username/notifications/:filter" => "users#show", constraints: { username: RouteFormat.username }
-    delete "#{root_path}/:username" => "users#destroy", constraints: { username: RouteFormat.username }
+    delete "#{root_path}/:username" => "users#destroy", constraints: { username: RouteFormat.username, format: /(json|html)/ }
     get "#{root_path}/by-external/:external_id" => "users#show", constraints: { external_id: /[^\/]+/ }
     get "#{root_path}/:username/flagged-posts" => "users#show", constraints: { username: RouteFormat.username }
     get "#{root_path}/:username/deleted-posts" => "users#show", constraints: { username: RouteFormat.username }
@@ -475,18 +472,18 @@ Discourse::Application.routes.draw do
   end
 
   get "user-badges/:username.json" => "user_badges#username", constraints: { username: RouteFormat.username }, defaults: { format: :json }
-  get "user-badges/:username" => "user_badges#username", constraints: { username: RouteFormat.username }
+  get "user-badges/:username" => "user_badges#username", constraints: { username: RouteFormat.username, format: /(json|html)/ }
 
   post "user_avatar/:username/refresh_gravatar" => "user_avatars#refresh_gravatar", constraints: { username: RouteFormat.username }
-  get "letter_avatar/:username/:size/:version.png" => "user_avatars#show_letter", constraints: { hostname: /[\w\.-]+/, size: /\d+/, username: RouteFormat.username, format: :png }
-  get "user_avatar/:hostname/:username/:size/:version.png" => "user_avatars#show", constraints: { hostname: /[\w\.-]+/, size: /\d+/, username: RouteFormat.username, format: :png }
+  get "letter_avatar/:username/:size/:version.png" => "user_avatars#show_letter", format: false, constraints: { hostname: /[\w\.-]+/, size: /\d+/, username: RouteFormat.username }
+  get "user_avatar/:hostname/:username/:size/:version.png" => "user_avatars#show", format: false, constraints: { hostname: /[\w\.-]+/, size: /\d+/, username: RouteFormat.username }
 
-  get "letter_avatar_proxy/:version/letter/:letter/:color/:size.png" => "user_avatars#show_proxy_letter", constraints: { format: :png }
+  get "letter_avatar_proxy/:version/letter/:letter/:color/:size.png" => "user_avatars#show_proxy_letter"
 
-  get "svg-sprite/:hostname/svg-:theme_ids-:version.js" => "svg_sprite#show", constraints: { hostname: /[\w\.-]+/, version: /\h{40}/, theme_ids: /([0-9]+(,[0-9]+)*)?/, format: :js }
+  get "svg-sprite/:hostname/svg-:theme_ids-:version.js" => "svg_sprite#show", format: false, constraints: { hostname: /[\w\.-]+/, version: /\h{40}/, theme_ids: /([0-9]+(,[0-9]+)*)?/ }
   get "svg-sprite/search/:keyword" => "svg_sprite#search", format: false, constraints: { keyword: /[-a-z0-9\s\%]+/ }
 
-  get "highlight-js/:hostname/:version.js" => "highlight_js#show", constraints: { hostname: /[\w\.-]+/, format: :js }
+  get "highlight-js/:hostname/:version.js" => "highlight_js#show", format: false, constraints: { hostname: /[\w\.-]+/ }
 
   get "stylesheets/:name.css.map" => "stylesheets#show_source_map", constraints: { name: /[-a-z0-9_]+/ }
   get "stylesheets/:name.css" => "stylesheets#show", constraints: { name: /[-a-z0-9_]+/ }
@@ -502,10 +499,10 @@ Discourse::Application.routes.draw do
   # used to download attachments
   get "uploads/:site/original/:tree:sha(.:extension)" => "uploads#show", constraints: { site: /\w+/, tree: /([a-z0-9]+\/)+/i, sha: /\h{40}/, extension: /[a-z0-9\.]+/i }
   # used to download attachments (old route)
-  get "uploads/:site/:id/:sha" => "uploads#show", constraints: { site: /\w+/, id: /\d+/, sha: /\h{16}/, format: /.*/ }
+  get "uploads/:site/:id/:sha" => "uploads#show", constraints: { site: /\w+/, id: /\d+/, sha: /\h{16}/ }
 
-  get "posts" => "posts#latest", id: "latest_posts", constraints: { format: /(json|rss)/ }
-  get "private-posts" => "posts#latest", id: "private_posts", constraints: { format: /(json|rss)/ }
+  get "posts" => "posts#latest", id: "latest_posts"
+  get "private-posts" => "posts#latest", id: "private_posts"
   get "posts/by_number/:topic_id/:post_number" => "posts#by_number"
   get "posts/by-date/:topic_id/:date" => "posts#by_date"
   get "posts/:id/reply-history" => "posts#reply_history"
@@ -615,7 +612,7 @@ Discourse::Application.routes.draw do
   resources :user_actions
 
   resources :badges, only: [:index]
-  get "/badges/:id(/:slug)" => "badges#show", constraints: { format: /(json|html|rss)/ }
+  get "/badges/:id(/:slug)" => "badges#show"
   resources :user_badges, only: [:index, :create, :destroy]
 
   get '/c', to: redirect(relative_url_root + 'categories')
@@ -656,7 +653,7 @@ Discourse::Application.routes.draw do
   end
 
   Discourse.filters.each do |filter|
-    get "#{filter}" => "list##{filter}"
+    get "#{filter}" => "list##{filter}", constraints: { format: /(json|html)/ }
     get "c/:category/l/#{filter}" => "list#category_#{filter}", as: "category_#{filter}"
     get "c/:category/none/l/#{filter}" => "list#category_none_#{filter}", as: "category_none_#{filter}"
     get "c/:parent_category/:category/l/#{filter}" => "list#parent_category_category_#{filter}", as: "parent_category_category_#{filter}"
@@ -689,11 +686,11 @@ Discourse::Application.routes.draw do
   get "topics/feature_stats"
 
   scope "/topics", username: RouteFormat.username do
-    get "created-by/:username" => "list#topics_by", as: "topics_by", defaults: { format: :json }
-    get "private-messages/:username" => "list#private_messages", as: "topics_private_messages", defaults: { format: :json }
-    get "private-messages-sent/:username" => "list#private_messages_sent", as: "topics_private_messages_sent", defaults: { format: :json }
-    get "private-messages-archive/:username" => "list#private_messages_archive", as: "topics_private_messages_archive", defaults: { format: :json }
-    get "private-messages-unread/:username" => "list#private_messages_unread", as: "topics_private_messages_unread", defaults: { format: :json }
+    get "created-by/:username" => "list#topics_by", as: "topics_by", constraints: { format: /(json|html)/ }, defaults: { format: :json }
+    get "private-messages/:username" => "list#private_messages", as: "topics_private_messages", constraints: { format: /(json|html)/ }, defaults: { format: :json }
+    get "private-messages-sent/:username" => "list#private_messages_sent", as: "topics_private_messages_sent", constraints: { format: /(json|html)/ }, defaults: { format: :json }
+    get "private-messages-archive/:username" => "list#private_messages_archive", as: "topics_private_messages_archive", constraints: { format: /(json|html)/ }, defaults: { format: :json }
+    get "private-messages-unread/:username" => "list#private_messages_unread", as: "topics_private_messages_unread", constraints: { format: /(json|html)/ }, defaults: { format: :json }
     get "private-messages-tags/:username/:tag_id.json" => "list#private_messages_tag", as: "topics_private_messages_tag", constraints: StaffConstraint.new
     get "groups/:group_name" => "list#group_topics", as: "group_topics", group_name: RouteFormat.username
 
@@ -804,8 +801,8 @@ Discourse::Application.routes.draw do
     get "/service-worker.js" => "static#service_worker_asset", format: :js
   end
 
-  get "cdn_asset/:site/*path" => "static#cdn_asset", format: false, constraints: { format: /.*/ }
-  get "brotli_asset/*path" => "static#brotli_asset", format: false, constraints: { format: /.*/ }
+  get "cdn_asset/:site/*path" => "static#cdn_asset", format: false
+  get "brotli_asset/*path" => "static#brotli_asset", format: false
 
   get "favicon/proxied" => "static#favicon", format: false
 
@@ -814,7 +811,7 @@ Discourse::Application.routes.draw do
   get "offline.html" => "offline#index"
   get "manifest.webmanifest" => "metadata#manifest", as: :manifest
   get "manifest.json" => "metadata#manifest"
-  get "opensearch" => "metadata#opensearch", constraints: { format: :xml }
+  get "opensearch" => "metadata#opensearch", format: :xml
 
   scope "/tags" do
     get '/' => 'tags#index'
@@ -883,5 +880,5 @@ Discourse::Application.routes.draw do
   resources :csp_reports, only: [:create]
 
   get "*url", to: 'permalinks#show', constraints: PermalinkConstraint.new
-  end
+
 end
diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb
index 0d39c338b60..ed177c2fff7 100644
--- a/spec/requests/users_controller_spec.rb
+++ b/spec/requests/users_controller_spec.rb
@@ -2372,12 +2372,12 @@ describe UsersController do
 
   describe '#confirm_admin' do
     it "fails without a valid token" do
-      get "/u/confirm-admin/invalid-token.json"
+      get "/u/confirm-admin/invalid-token.josn"
       expect(response).not_to be_successful
     end
 
     it "fails with a missing token" do
-      get "/u/confirm-admin/a0a0a0a0a0.json"
+      get "/u/confirm-admin/a0a0a0a0a0.josn"
       expect(response).to_not be_successful
     end
 
@@ -2385,7 +2385,7 @@ describe UsersController do
       user = Fabricate(:user)
       ac = AdminConfirmation.new(user, Fabricate(:admin))
       ac.create_confirmation
-      get "/u/confirm-admin/#{ac.token}.json"
+      get "/u/confirm-admin/#{ac.token}.josn"
       expect(response.status).to eq(200)
 
       user.reload
@@ -2398,7 +2398,7 @@ describe UsersController do
 
       ac = AdminConfirmation.new(user, admin)
       ac.create_confirmation
-      get "/u/confirm-admin/#{ac.token}.json", params: { token: ac.token }
+      get "/u/confirm-admin/#{ac.token}.josn", params: { token: ac.token }
       expect(response.status).to eq(200)
 
       user.reload
@@ -2411,7 +2411,7 @@ describe UsersController do
 
       ac = AdminConfirmation.new(user, Fabricate(:admin))
       ac.create_confirmation
-      get "/u/confirm-admin/#{ac.token}.json"
+      get "/u/confirm-admin/#{ac.token}.josn"
       expect(response).to_not be_successful
 
       user.reload
@@ -2423,7 +2423,7 @@ describe UsersController do
         user = Fabricate(:user)
         ac = AdminConfirmation.new(user, Fabricate(:admin))
         ac.create_confirmation
-        post "/u/confirm-admin/#{ac.token}.json"
+        post "/u/confirm-admin/#{ac.token}.josn"
         expect(response.status).to eq(200)
 
         user.reload