.topmsg

   1 Subject: [PATCH] gitweb: support caching projects list
   2
   3 On repo.or.cz (permanently I/O overloaded and hosting 1050 project +
   4 forks), the projects list (the default gitweb page) can take more than
   5 a minute to generate. This naive patch adds simple support for caching
   6 the projects list data structure so that all the projects do not need
   7 to get rescanned at every page access.
   8
   9 $projlist_cache_lifetime gitweb configuration variable is introduced,
  10 by default set to zero. If set to non-zero, it describes the number of
  11 minutes for which the cache remains valid. Only single project root
  12 per system can use the cache. Any script running with the same uid as
  13 gitweb can change the cache trivially - this is for secure
  14 installations only.
  15
  16 The cache itself is stored in $cache_dir/$projlist_cache_name using
  17 Storable to store() Perl data structure with the list of project
  18 details.  When reusing the cache, the data is retrieve()'d back into
  19 @projects.
  20
  21 To prevent contention when multiple accesses coincide with cache
  22 expiration, the timeout is postponed to time()+120 when we start
  23 refreshing.  When showing cached version, a disclaimer is shown
  24 at the top of the projects list.
  25
  26 [jn: moved from Data::Dumper to Storable for serialization of data]
  27
  28 $cache_grpshared gitweb configuration variable can be set to 1 to
  29 create the cache file group-readable and group-writable to facilitate
  30 external re-generation of the cache.
  31
  32 Signed-off-by: Petr Baudis <pasky@ucw.cz>
  33 Signed-off-by: Jakub Narebski <jnareb@gmail.com>
  34 Signed-off-by: Kyle J. McKay <mackyle@gmail.com>
  35
  36 ---
  37
  38 This version uses Storable (which gives binary output), instead of a
  39 bit unsafe, because of need to 'eval' to unserialize, Data::Dumper.
  40
  41 It should also be a bit faster: unscientific comparison gives that
  42 Storable is 3-4 times faster than using Data::Dumper on totally
  43 artificial data of 1050 elements table of hashes.
  44
  45 Below replies to comments for first (or second if you count original
  46 Pasky version) version of this patch.
  47
  48 No interdiff there... on the other hand it includes gitweb/README
  49
  50 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
  51
  52 [The message below didn't made it to git mailing list due to not
  53 quoting J.H. name in email: it should be
  54   "J.H." <warthog19@eaglescrag.net>
  55 not
  56    J.H. <warthog19@eaglescrag.net>]
  57
  58 On Mon, 17 Mar 2008, Rafael Garcia-Suarez wrote:
  59 > On 17/03/2008, Jakub Narebski <jnareb@gmail.com> wrote:
  60 >>
  61 >>  The cache itself is stored in /tmp/gitweb.index.cache as a
  62 >>  Data::Dumper dump of the perl data structure with the list of project
  63 >>  details.  When reusing the cache, the file is simply eval'd back into
  64 >>  @projects.
  65 >
  66 > Correct me if I'm wrong (I haven't read the full source code...) Doesn't
  67 > that mean that anyone who can write this file can make gitweb eval
  68 > arbitrary perl code? And anyone can write a file in /tmp, usually... If
  69 > so, I would strongly advise against including this patch in its current
  70 > state, not without at least checking the ownership of the cache file.
  71
  72 First, this patch is an RFC, mainly meant as better version of Pasky's
  73 patch with (the nearly) the same subject, to help repo.or.cz, when it
  74 can be assumed that installation is secure.
  75
  76 Second, it was changed in this version of patch: gitweb now uses
  77 Storable to write Perl data to disk and read it back, so there is no
  78 need to eval to read back the data from cache, as it was when using
  79 Data::Dumper package.  Additionally you can now define where to store
  80 cache; by default it is /tmp/gitweb/, and you can make gitweb write
  81 only for projects with gitweb uid (if directory does not exists,
  82 gitweb would create it if possible with appropriate permissions).
  83
  84 >>  Note: instead of using Data::Dumper to serialize data we could use
  85 >>  Storable module (distributed with Perl like Data::Dumper).  From what
  86 >>  I've checked it has larger initial cost, but might be better for
  87 >>  larger number of projects, exactly the situation when projects list
  88 >>  caching is needed.
  89 >
  90 > Storable would be more secure. (But check file ownership!)
  91
  92 This version uses Storable, which should be more secure (but see
  93 below)... and additionally bit faster.
  94
  95 For the simplicity of patch gitweb does not check cache file
  96 ownership, nor permissions on cache file and on directory it is in,
  97 but this should be fairly easy to add.
  98
  99 BTW. store() and retrieve() from Storable "die" on many errors;
 100 in unsafe environment we would probably want to protect gitweb from
 101 them failing due to incorrect file format, or nonreadable cache file,
 102 etc.
 103
 104 > I don't know how complex are the data structures you're going to
 105 > save.
 106
 107 It is array (list) or array reference (but ordering of items doesn't
 108 matter) of records (hashes / hash references). Fields can be integer,
 109 string (possibly containing quite characters and other strangeness),
 110 or undef.
 111
 112 > My paranoid preference would be to use a text-only format. If CPAN
 113 > modules are out, devise your own dump/load routines, maybe?
 114
 115 What format should it be? Data::Dumper format looks like this:
 116
 117   [
 118           {
 119             'owner' => 'Jakub Narebski',
 120             'descr_long' => './t9500-gitweb-standalone-no-errors.sh test repository',
 121             'mtime' => undef,
 122             'age_string' => '7 days ago',
 123             'path' => 'trash.git',
 124             'age' => 652787,
 125             'descr' => './t9500-gitweb-standalone... '
 126           },
 127           {
 128             'owner' => 'Jakub Narebski',
 129             'descr_long' => "git with Jakub Narebski modifications, local copy.",
 130             'mtime' => undef,
 131             'age_string' => '80 min ago',
 132             'path' => 'git.git',
 133             'age' => 4813,
 134             'descr' => 'git with Jakub Narebski modif... '
 135           }
 136   ]
 137
 138 Another solution would be to use YAML [Tiny], but although Perl
 139 packages for YAML are in contrib repositories, they are not installed
 140 by default with Perl, and most probably are not present on the
 141 system. Additonally at least YAML::Tiny is slower than even
 142 Data::Dumper when retrieving data, around 4-5 times slower... on the
 143 other hand YAML::Tiny is Pure Perl implementation; other solutions
 144 which use C libraries should be much faster.
 145
 146 YAML output looks like this:
 147
 148   ---
 149   -
 150     age: 652787
 151     age_string: '7 days ago'
 152     descr: './t9500-gitweb-standalone... '
 153     descr_long: './t9500-gitweb-standalone-no-errors.sh test repository'
 154     mtime: ~
 155     owner: 'Jakub Narebski'
 156     path: trash.git
 157   -
 158     age: 4813
 159     age_string: '80 min ago'
 160     descr: 'git with Jakub Narebski modif... '
 161     descr_long: 'git with Jakub Narebski modifications, local copy.'
 162     mtime: ~
 163     owner: 'Jakub Narebski'
 164     path: git.git
 165
 166 Yet another solution would be to use [much] restricted version of
 167 ini-like git config format, and perhaps reuse git-cvsserver parsing of
 168 this format, or simply call "git-config --file <file> -z -l" and
 169 gitweb code to parse git-config output.
 170
 171 The output whould then look like this:
 172
 173   [gitweb "trash.git"]
 174         age = 652787
 175         age_string = "7 days ago"
 176         descr = './t9500-gitweb-standalone... '
 177         descr_long = "./t9500-gitweb-standalone-no-errors.sh test repository"
 178         mtime
 179         owner = Jakub Narebski
 180
 181   [gitweb "git.git"]
 182         age = 4813
 183         age_string = "80 min ago"
 184         descr = "git with Jakub Narebski modif... "
 185         descr_long = "git with Jakub Narebski modifications, local copy."
 186         mtime
 187         owner = Jakub Narebski
 188
 189 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 190
 191 On Mon, 17 Mar 2008, Frank Lichtenheld wrote:
 192 > On Mon, Mar 17, 2008 at 04:09:29PM +0100, Jakub Narebski wrote:
 193 >>
 194 >> From: Petr Baudis <pasky@suse.cz>
 195 >> $projlist_cache_lifetime gitweb configuration variable is introduced,
 196 >> by default set to zero. If set to non-zero, it describes the number of
 197 >> minutes for which the cache remains valid. Only single project root
 198 >> per system can use the cache. Any script running with the same uid as
 199 >> gitweb can change the cache trivially - this is for secure
 200 >> installations only.
 201 >
 202 > The more subtle threat is the fact that anyone with writing
 203 > rights to /tmp can give gitweb any data he wants if the file doesn't
 204 > exist yet.
 205
 206 Right.
 207
 208 > At the very least you should:
 209 >
 210 >  - Allow to override /tmp (via ENV{TMPDIR} or via a configuration
 211 >    variable)
 212
 213 Done.
 214
 215 >  - Advise people to change that to something that is not
 216 >    world-writable
 217
 218 I wrote a bit about this in gitweb/README, feel free to add to it.
 219 Additionally gitweb can create cache directory if it does not exist,
 220 and it does so with restrictive premissions.
 221
 222 >  - Check if the file is owned by the uid gitweb is running under and
 223 >    not word-writable.
 224
 225 Currently not done, to make patch simpler; should be fairly easy to
 226 add. I guess it would be best to create is_cache_safe() or something
 227 like that subroutine.
 228
 229 > [...]
 230 >> +    my @projects;
 231 >> +    my $stale = 0;
 232 >> +    my $now = time();
 233 >> +    if ($cache_lifetime && -f $cache_file &&
 234 >> +        stat($cache_file)->mtime + $cache_lifetime * 60 > $now &&
 235 >> +        open(my $fd, '<', $cache_file)) {
 236 >> +            $stale = $now - stat($cache_file)->mtime;
 237 >
 238 > One stat() call instead of three would be better for performance.
 239
 240 Thanks. Done.