Commit 47800925938c0dd88e1d45210eb6a139217c78bd
1 parent
03e31678
Exists in
master
and in
29 other branches
Test the existence of theme.yml before reading
Prevents themes page to crash when approved_themes() finds a non theme entity inside themes dir. Closes ActionItem2269
Showing
2 changed files
with
15 additions
and
2 deletions
Show diff stats
app/models/theme.rb
@@ -43,8 +43,13 @@ class Theme | @@ -43,8 +43,13 @@ class Theme | ||
43 | 43 | ||
44 | def approved_themes(owner) | 44 | def approved_themes(owner) |
45 | Dir.glob(File.join(system_themes_dir, '*')).select do |item| | 45 | Dir.glob(File.join(system_themes_dir, '*')).select do |item| |
46 | - config = YAML.load_file(File.join(item, 'theme.yml')) | ||
47 | - (config['owner_type'] == owner.class.base_class.name) && (config['owner_id'] == owner.id) || config['public'] | 46 | + if File.exists?( File.join(item, 'theme.yml') ) |
47 | + config = YAML.load_file(File.join(item, 'theme.yml')) | ||
48 | + (config['owner_type'] == owner.class.base_class.name) && | ||
49 | + (config['owner_id'] == owner.id) || config['public'] | ||
50 | + else | ||
51 | + false | ||
52 | + end | ||
48 | end.map do |desc| | 53 | end.map do |desc| |
49 | new(File.basename(desc)) | 54 | new(File.basename(desc)) |
50 | end | 55 | end |
test/unit/theme_test.rb
@@ -170,6 +170,14 @@ class ThemeTest < ActiveSupport::TestCase | @@ -170,6 +170,14 @@ class ThemeTest < ActiveSupport::TestCase | ||
170 | assert ! Theme.approved_themes(profile).include?(Theme.find(t3.id)) | 170 | assert ! Theme.approved_themes(profile).include?(Theme.find(t3.id)) |
171 | end | 171 | end |
172 | 172 | ||
173 | + should 'not list non theme files or dirs inside themes dir' do | ||
174 | + Theme.stubs(:system_themes_dir).returns(TMP_THEMES_DIR) | ||
175 | + Dir.mkdir(TMP_THEMES_DIR) | ||
176 | + Dir.mkdir(TMP_THEMES_DIR+'/empty-dir') | ||
177 | + File.new(TMP_THEMES_DIR+'/my-logo.png', File::CREAT) | ||
178 | + assert Theme.approved_themes(Environment.default).empty? | ||
179 | + end | ||
180 | + | ||
173 | should 'set theme to public' do | 181 | should 'set theme to public' do |
174 | t = Theme.new('mytheme') | 182 | t = Theme.new('mytheme') |
175 | t.public = true | 183 | t.public = true |