FIX: apply defaults constraints to routes format (#7890)

This fixes the problem where if a route ends with a dynamic segment and the segment contains a period e.g. `my.name`, `name` is interpreted as the format. This applies a default format constraints `/(json|html)/` on all routes. If you'd like a route to have a different format constraints, you can do something like this:

```ruby
get "your-route" => "your_controlller#method", constraints: { format: /(rss|xml)/ }

#or
get "your-route" => "your_controlller#method", constraints: { format: :xml }
```
This commit is contained in:
Osama Sayegh 2019-07-15 21:55:11 +03:00 committed by GitHub
parent 6515ff19e5
commit 7d01c5de1a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 53 additions and 47 deletions

View File

@ -1065,7 +1065,10 @@ class UsersController < ApplicationController
@confirmed = true
end
render layout: 'no_ember'
respond_to do |format|
format.json { render json: success_json }
format.html { render layout: 'no_ember' }
end
end
def list_second_factors

View File

@ -12,7 +12,8 @@ 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
relative_url_root = (defined?(Rails.configuration.relative_url_root) && Rails.configuration.relative_url_root) ? Rails.configuration.relative_url_root + '/' : '/'
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 + '/' : '/'
match "/404", to: "exceptions#not_found", via: [:get, :post]
get "/404-body" => "exceptions#not_found_body"
@ -24,13 +25,15 @@ Discourse::Application.routes.draw do
post "webhooks/sendgrid" => "webhooks#sendgrid"
post "webhooks/sparkpost" => "webhooks#sparkpost"
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
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
end
resources :about do
@ -350,17 +353,17 @@ Discourse::Application.routes.draw do
post "composer/parse_html" => "composer#parse_html"
resources :static
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)/ }
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"
%w{guidelines rules conduct}.each do |faq_alias|
get faq_alias => "static#show", id: "guidelines", as: faq_alias, constraints: { format: /(json|html)/ }
get faq_alias => "static#show", id: "guidelines", as: faq_alias
end
get "my/*path", to: 'users#my_redirect'
@ -420,8 +423,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, 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" => "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/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 }
@ -463,7 +466,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, format: /(json|html)/ }
delete "#{root_path}/:username" => "users#destroy", constraints: { username: RouteFormat.username }
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 }
@ -472,18 +475,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, format: /(json|html)/ }
get "user-badges/:username" => "user_badges#username", constraints: { username: RouteFormat.username }
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", 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/: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_proxy/:version/letter/:letter/:color/:size.png" => "user_avatars#show_proxy_letter"
get "letter_avatar_proxy/:version/letter/:letter/:color/:size.png" => "user_avatars#show_proxy_letter", constraints: { format: :png }
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/: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/search/:keyword" => "svg_sprite#search", format: false, constraints: { keyword: /[-a-z0-9\s\%]+/ }
get "highlight-js/:hostname/:version.js" => "highlight_js#show", format: false, constraints: { hostname: /[\w\.-]+/ }
get "highlight-js/:hostname/:version.js" => "highlight_js#show", constraints: { hostname: /[\w\.-]+/, format: :js }
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_]+/ }
@ -499,10 +502,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}/ }
get "uploads/:site/:id/:sha" => "uploads#show", constraints: { site: /\w+/, id: /\d+/, sha: /\h{16}/, format: /.*/ }
get "posts" => "posts#latest", id: "latest_posts"
get "private-posts" => "posts#latest", id: "private_posts"
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/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"
@ -612,7 +615,7 @@ Discourse::Application.routes.draw do
resources :user_actions
resources :badges, only: [:index]
get "/badges/:id(/:slug)" => "badges#show"
get "/badges/:id(/:slug)" => "badges#show", constraints: { format: /(json|html|rss)/ }
resources :user_badges, only: [:index, :create, :destroy]
get '/c', to: redirect(relative_url_root + 'categories')
@ -653,7 +656,7 @@ Discourse::Application.routes.draw do
end
Discourse.filters.each do |filter|
get "#{filter}" => "list##{filter}", constraints: { format: /(json|html)/ }
get "#{filter}" => "list##{filter}"
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}"
@ -686,11 +689,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", 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 "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 "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
@ -801,8 +804,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
get "brotli_asset/*path" => "static#brotli_asset", format: false
get "cdn_asset/:site/*path" => "static#cdn_asset", format: false, constraints: { format: /.*/ }
get "brotli_asset/*path" => "static#brotli_asset", format: false, constraints: { format: /.*/ }
get "favicon/proxied" => "static#favicon", format: false
@ -811,7 +814,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", format: :xml
get "opensearch" => "metadata#opensearch", constraints: { format: :xml }
scope "/tags" do
get '/' => 'tags#index'
@ -880,5 +883,5 @@ Discourse::Application.routes.draw do
resources :csp_reports, only: [:create]
get "*url", to: 'permalinks#show', constraints: PermalinkConstraint.new
end
end

View File

@ -2372,12 +2372,12 @@ describe UsersController do
describe '#confirm_admin' do
it "fails without a valid token" do
get "/u/confirm-admin/invalid-token.josn"
get "/u/confirm-admin/invalid-token.json"
expect(response).not_to be_successful
end
it "fails with a missing token" do
get "/u/confirm-admin/a0a0a0a0a0.josn"
get "/u/confirm-admin/a0a0a0a0a0.json"
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}.josn"
get "/u/confirm-admin/#{ac.token}.json"
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}.josn", params: { token: ac.token }
get "/u/confirm-admin/#{ac.token}.json", 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}.josn"
get "/u/confirm-admin/#{ac.token}.json"
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}.josn"
post "/u/confirm-admin/#{ac.token}.json"
expect(response.status).to eq(200)
user.reload