Description
The bug is similar to MoinMoinBugs/RenameToDeletedPage. But it already happens if a user has previewed a non-existing page before.
Steps to reproduce
- Choose a non-existing page "foo".
- Select "edit" and then "preview" but do not save it.
- Close the window and choose another page you want to rename, e.g. "bar".
- Try to rename "bar" to "foo".
The result is that the wiki says that this page already exists.
Example
Reproduce it in your wiki.
Component selection
- general
Details
This Wiki.
Workaround
- Create the page "foo" and save it.
- Rename the page to "Trash/foo".
- Delete "Trash/foo".
- Rename "bar" to "foo".
Discussion
The most annoying problem is: Some user can be very very nasty and just preview some pages. It's not logged as "last changes" or something so you do not see that a user has done this. But you have a lot of work to do with renaming these sites to the trash...
The problem is that non existing pages are created by editing them, because edited pages have edit lock, and the lock is saved inside the page directory. Because moin does not know what the user did with the edited page, the page directory is never removed.
An attacker can create thousands of pages on a wiki using a script using action=edit on with random page names from a dictionary. Those empty page directories will slow down all operations that needs a page lists, and prevent renaming of pages in the future.
The solution is simple: don't use edit locks for non existing pages and create the page directory only when a user save changes in the first time. There is very little chance that multiple users will create the same new page in the same time, and if they do, they will have the standard edit conflict anyway.
This is another issue related to using non safe operation on GET requests. A GET request should not change your database in any observable way.
Remove empty pages when checking if page exists
Here is an idea for 1.6dev how to get rid of this edit-lock files and its pagedir by extending Page.exist. We do take care about the lock time. Of course we can split some of the code into a purge function. -- ReimarBauer 2007-05-17 22:33:09
Page exists is too expensive as is - adding even more file system access in this point is not a good idea. A solution should avoid using page directory for locking, so a page directory is created only when saving a page. For example, keeps locks in data/pages/_locks/<pagename>.
- I know and Page will be refactored during SOC. If it's possible to get solved this problem with small changes of the current 1.6 code it will be only a temporary solution.
--- a/MoinMoin/Page.py Wed May 16 15:01:44 2007 +0200 +++ b/MoinMoin/Page.py Fri May 18 22:59:39 2007 +0200 @@ -646,7 +646,28 @@ class Page(object): for use_underlay in checklist: pagedir = self.getPagePath(use_underlay=use_underlay, check_create=0) if os.path.exists(pagedir): - return True + dir = os.listdir(pagedir) + if not dir: + return False + # check if its a real page + elif ('current' in dir or + 'edit-log' in dir or + 'attachments' in dir or + 'revisions' in dir): + return True + # check if its a preview created pagedir only + elif 'edit-lock' in dir: + self.lock._readLockFile() + if self.lock.now > (wikiutil.version2timestamp(self.lock.ed_time_usecs) + self.lock.timeout): + # edit-lock is expired and the whole dir could be purged + import shutil + try: + shutil.rmtree(pagedir) + return False + except OSError, err: + return True + + return True return False else: # Look for non-deleted pages only, using get_rev diff -r 6d1d337c5a76 MoinMoin/PageEditor.py --- a/MoinMoin/PageEditor.py Wed May 16 15:01:44 2007 +0200 +++ b/MoinMoin/PageEditor.py Fri May 18 00:19:40 2007 +0200 @@ -1203,7 +1203,7 @@ class PageLock: timestamp = self.request.user.getFormattedDateTime(self.timestamp) owner = self.owner_html secs_valid = self.timestamp + self.timeout - self.now # do we own the lock, or is it stale? if self.owner is None or self.uid == self.owner or secs_valid < 0: # create or bump the lock @@ -1230,7 +1230,7 @@ class PageLock: )) result = 1, '\n'.join(msg) else: - mins_valid = (secs_valid+59) / 60 + mins_valid = (secs_valid + 59) / 60 if self.locktype == 'lock': # lout out user result = 0, _( @@ -1274,6 +1274,7 @@ To leave the editor, press the Cancel bu self.owner = None self.owner_html = wikiutil.escape(_("<unknown>")) self.timestamp = 0 + self.ed_time_usecs = 0 if self.locktype: try: @@ -1284,6 +1285,7 @@ To leave the editor, press the Cancel bu if entry: self.owner = entry.userid or entry.addr self.owner_html = entry.getEditor(self.request) + self.ed_time_usecs = entry.ed_time_usecs self.timestamp = wikiutil.version2timestamp(entry.ed_time_usecs)
Discussion
This is a somewhat obscure trigger, but no less valid. There is a much more common way to hit this problem, which I'm pretty sure has the same root cause. Just wanted to offer it as another thing to consider:
- User renames "foo" to "bar" because it seems like a better name
- User discovers that "foo" was the best name after all.
- User tries to simply rename "bar" back to "foo", but is denied with a message saying that "foo" already exists so it can't rename
- Look at the page "foo", and it appears to not exist, and looking at the history shows no edits. (This is because the history moved with the page when it got renamed the first time.
The only workaround I could come up with was to create a new page "foo", and copy-paste the contents from "bar", then delete "bar". The problem with this is that all of foo's history is sitting over at the deleted page bar. Will this be fixed as part of the 1.6 changes? -- SteveDavison 2007-09-12 13:50:15
Plan
- Priority:
- Assigned to:
- Status: