Description

If one uses in a theme user.save() a user can fire this function very fast by often reloading a page.

It happens to me that my account becomes corrupted on the filesystem. The enc_password was removed. My username and settings vanishes from the theme. I was still logged in but with logging out I was doomed.

Steps to reproduce

I found it by the solenoid theme. There are additional user settings defined. solenoid_userprefs = True in wikiconfig enables these vars. You should backup your id file. Then login. You need 5 to 10 times fast reloading (shift reload) of a page to get your account broken.

Example

Component selection

  • user.save

Details

MoinMoin Version

1.9

OS and Version

Python Version

Server Setup

Server Details

Language you are using the wiki in (set in the browser/UserPreferences)

Workaround

don't use user.save() on places where you can fire it very fast.

Discussion

Looks like we need locking here. Usually we use it in moin with form based data submission.

Could be solved by following patch. Does it harm to have tmp files in the users dir?

 1 diff -r 172146fe48a2 MoinMoin/user.py
 2 --- a/MoinMoin/user.py	Tue May 11 23:08:11 2010 +0200
 3 +++ b/MoinMoin/user.py	Sun May 16 19:01:30 2010 +0200
 4 @@ -19,12 +19,12 @@
 5  @license: GNU GPL, see COPYING for details.
 6  """
 7  
 8 -import os, time, codecs, base64
 9 +import os, time, codecs, base64, tempfile
 10  
 11  from MoinMoin.support.python_compatibility import hash_new, hmac_new
 12  
 13  from MoinMoin import config, caching, wikiutil, i18n, events
 14 -from MoinMoin.util import timefuncs, random_string
 15 +from MoinMoin.util import filesys, timefuncs, random_string
 16  from MoinMoin.wikiutil import url_quote_plus
 17  
 18  
 19 @@ -542,11 +542,8 @@
 20  os.makedirs(user_dir)
 21  
 22  self.last_saved = str(time.time())
 23 -
 24 - # !!! should write to a temp file here to avoid race conditions,
 25 - # or even better, use locking
 26 -
 27 - data = codecs.open(self.__filename(), "w", config.charset)
 28 + fd, _tmp_filename = tempfile.mkstemp('.tmp', self.__filename(), self._cfg.user_dir)
 29 + data = codecs.open(_tmp_filename, "w", config.charset)
 30  data.write("# Data saved '%s' for id '%s'\n" % (
 31  time.strftime(self._cfg.datetime_fmt, time.localtime(time.time())),
 32  self.id))
 33 @@ -565,6 +562,7 @@
 34  line = line.replace('\n', ' ').replace('\r', ' ') # no lineseps
 35  data.write(line + '\n')
 36  data.close()
 37 + filesys.rename(_tmp_filename, self.__filename())
 38  
 39  arena = 'user'
 40  key = 'name2id'
user.patch

Issues with patch:

  • don't use self.__filename() for prefix (it contains full path), just use default prefix

  • for opening, try to use fd (already open file handle, you can use fdopen() to open a python file), not the filename

Plan

  • Priority:
  • Assigned to:
  • Status:


CategoryMoinMoinBug

MoinMoin: MoinMoinBugs/NeedLockingForSavingUserData (last edited 2010年06月03日 22:42:38 by ThomasWaldmann )

AltStyle によって変換されたページ (->オリジナル) /