Skip to content

New hset_redis_diff_cache returns unsafe HTML string (showing wrong diff information at Changes tab)

Problem

After enabling hset_redis_diff_cache I noticed a few MRs loading as the following:

Screen_Shot_2019-12-09_at_13.14.41

Also, after checking Redis directly, it doesn't seem to have any file in place:

redis /Users/osw/projects/gitlab/gdk-ee/redis/redis.socket> get "highlighted-diff-files:merge_request_diffs/66-20191128195333571669:1:{:max_files=>1000, :max_lines=>50000, :ignore_whitespace_change=>false, :expanded=>false}"
(error) WRONGTYPE Operation against a key holding the wrong kind of value
redis /Users/osw/projects/gitlab/gdk-ee/redis/redis.socket> mget "highlighted-diff-files:merge_request_diffs/66-20191128195333571669:1:{:max_files=>1000, :max_lines=>50000, :ignore_whitespace_change=>false, :expanded=>false}"
1) (nil)
redis /Users/osw/projects/gitlab/gdk-ee/redis/redis.socket> mget "highlighted-diff-files:merge_request_diffs/66-20191128195333571669:1:{:max_files=>1000, :max_lines=>50000, :ignore_whitespace_change=>false, :expanded=>false}" "asdf"
1) (nil)
2) (nil)

If I try to fetch the cache directly, it raises an error:

[7] pry(main)> mr.diffs.send(:cache).send(:cached_content)

NoMethodError: undefined method `file_path' for #<Gitlab::Git::Diff:0x00007f972ef4f3e0>
from /Users/osw/projects/gitlab/gdk-ee/gitlab/lib/gitlab/git/diff_collection.rb:161:in `block in each_serialized_patch'

@kerrizor I'm feeling something was missed on the tests as it should be calling file_path at Diff::File, not Gitlab::Git::Diff.

Edited by Oswaldo Ferreira