FIX: Don't throw 500 for invalid website url input

It's possible to cause a 500 error by putting in weird characters in the
input field for updating a users website on their profile.

Normal invalid input like not including the domain extension is already
handled by the user_profile model validation. This fix ensures a server
error doesn't occur for weird input characters.
This commit is contained in:
Blake Erickson 2020-04-22 13:50:45 -06:00
parent 8adccaf98c
commit 9cbbaf4237
2 changed files with 29 additions and 15 deletions

View File

@ -149,25 +149,30 @@ class UserUpdater
saved = nil
User.transaction do
if attributes.key?(:muted_usernames)
update_muted_users(attributes[:muted_usernames])
end
begin
User.transaction do
if attributes.key?(:muted_usernames)
update_muted_users(attributes[:muted_usernames])
end
if attributes.key?(:ignored_usernames)
update_ignored_users(attributes[:ignored_usernames])
end
if attributes.key?(:ignored_usernames)
update_ignored_users(attributes[:ignored_usernames])
end
name_changed = user.name_changed?
if (saved = (!save_options || user.user_option.save) && user_profile.save && user.save) &&
(name_changed && old_user_name.casecmp(attributes.fetch(:name)) != 0)
name_changed = user.name_changed?
if (saved = (!save_options || user.user_option.save) && user_profile.save && user.save) &&
(name_changed && old_user_name.casecmp(attributes.fetch(:name)) != 0)
StaffActionLogger.new(@actor).log_name_change(
user.id,
old_user_name,
attributes.fetch(:name) { '' }
)
StaffActionLogger.new(@actor).log_name_change(
user.id,
old_user_name,
attributes.fetch(:name) { '' }
)
end
end
rescue Addressable::URI::InvalidURIError => e
# Prevent 500 for crazy url input
return saved
end
DiscourseEvent.trigger(:user_updated, user) if saved

View File

@ -416,6 +416,15 @@ describe UserUpdater do
end
end
context 'when website is invalid' do
it 'returns an error' do
user = Fabricate(:user)
updater = UserUpdater.new(acting_user, user)
expect(updater.update(website: 'ʔ<')).to eq nil
end
end
context 'when custom_fields is empty string' do
it "update is successful" do
user = Fabricate(:user)