Apache OpenOffice (AOO) Bugzilla – Full Text Issue Listing |
Summary: | ACL information not being read. | ||||||
---|---|---|---|---|---|---|---|
Product: | General | Reporter: | sirloxelroy <sirloxelroy> | ||||
Component: | code | Assignee: | thorsten.martens | ||||
Status: | CLOSED FIXED | QA Contact: | issues@framework <issues> | ||||
Severity: | Trivial | ||||||
Priority: | P3 | CC: | chris, issues, kpalagin, mail.twerner, Mathias_Bauer, noel, tim | ||||
Version: | OOO300m9 | ||||||
Target Milestone: | OOo 3.2 | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Issue Type: | DEFECT | Latest Confirmation in: | --- | ||||
Developer Difficulty: | --- | ||||||
Attachments: |
|
Description
sirloxelroy
2008-12-04 22:25:11 UTC
UPDATE: I tested the same files with Neo-Office (http://www.neooffice.org/) and they open up in RW mode. I am surprised that nothing has been done on this. If OpenOffice wants to become a player in Mac Office Environments someone should at least comment that they are going to look into this. I' ll have a look into this. Issue is confirmed. I tested with a file with permissions set to standard 0644 with owner fheckl:wheel (just saved the file into /Users/Shared). I set ACL permissions with chmod +a "staff allow read,write" to allow all users to write to this file. With a different user(tester:staff) account I got the following: - TextEdit let's you edit the file, but you cannot save actually (write gives an error) - OOo opens the file in read-only mode - NeoOffice (tested with version 2.2.4) let's you edit the file and correctly save any updates (write gives no error). Will add more tests over the weekend. This was an Issue with NeoOffice 3.0 Early Access 2.0. It turned out to be a core OO issue which Plube has written a patch for (Bug #3406). I just thought I would let you all know. Also, I am sorry if I sounded a little scathing about the issue, I should have waited for a little while after posting the comment. This is a general issue and the same thing happens with OOorg 3 and Linux and it is a major regression from OOorg 2.x. Our users can no longer write shared files on our filesystem that can be written by other applications including OOorg 2.x. The bug #31890 describes a similar problem. BTW there is no patch in Bug #3406 as the former poster indicated. This is not a Mac only issue. I have reproduced it by using extended ACLs, and I am told it can be reproduced via Samba too. The problem occurs in any place that the simple detection heuristic for a read only file in osl_getFileStatus fails. This is a result of the change in mediadescriptor.cxx (Revision 1.17.78.1) that was made as part of Issue 85794. Before the change, OOo would try to open the file for writing to decide if the file could be opened for editing or not. Now, it will not try to open the file for writing if locking is off, and the heuristic will be used instead. haggai->mav: I am assigning this to you for your comments about this change. I will attach a patch which solves the immediate problem. I have not yet verified if mav's locking changes are all working as intended. haggai->mav: Assigning to you. Please see my last comment. Created attachment 61384 [details]
Patch to restore the old behaviour when opening files with ACLs or remote files
The OOo3.1 version should work well in this scenario in default configuration, could somebody please try the OOo3.1 release candidate? mav->haggai: Thank you for the investigation. Sorry, but the attached patch is just a hack that would break the behavior in case system file locking is turned off. The real problem, as you already have already written, is in implementation/usage of osl_getFileStatus() call. This problem should be fixed even if the default configuration works well in OOo3.1, since it is possible to turn the system file locking off, and then we would have just the same problem. I tested this problem with OOo 3.1 rc1 (Build 9396) (on Debian Linux) and the problem is still there. I'm user "nk" and the file permissions (extended ACLs) are: # getfacl testACL.odt # file: testACL.odt # owner: root # group: root user::rw- user:nk:rw- group::r-- mask::rw- other::r-- OOo 3.1 rc1 opens this file - where I have write permission - read-only. What I mean is that if it does not work with the 3.1 rc1 it would not also work with the attached patch. The 3.1 in default configuration does exactly the same. Anyway, the detection of the file readonly state has to be fixed. mav->noelkoethe: Is it possible to create a new file on the location where the target file is? It not, it is probably the problem, OOo can not create the lock file. Sorry, a typo. I mean of course "...If not, it is probably the problem, OOo can not create the lock file." The directory is /tmp/ and everyone can write into the directory. If I'm the owner (or have writeaccess via group) the lockfile is created: -rw-r--r-- 1 nk root 76 15. Apr 16:51 .~lock.testACL.odt# -rw-rw-r--+ 1 root nk 7461 15. Apr 14:46 testACL.odt and if I have write access via the extended ACLs no lockfile will be created. The lockfile is only created if OOo detects the document as writeable which makes sense.:) haggai->mav: > mav->haggai: Thank you for the investigation. Sorry, but the attached patch is > just a hack that would break the behavior in case system file locking is turned > off. Okay, we hadn't run through the full set of tests yet to determine if anything else had broken as a result of the patch. That was why I posted the patch for your comments before going further :) > The real problem, as you already have already written, is in > implementation/usage of osl_getFileStatus() call. This problem should be fixed > even if the default configuration works well in OOo3.1, since it is > possible to turn the system file locking off, and then we would have just > the same problem. I don't think you can rely on osl_getFileStatus, instead of opening the file as writable. You'll always have potential race conditions, where the state of the file changes between the calls. Or are you thinking of using osl_getFileStatus to see if you could open it as writable, then handle an error in the race condition case if you then find it is no longer writeable? mav->noelkoethe: You are right. Sorry, I have forgot about the last change I did there. Indeed, the file readonly state is checked always now, so the patch would not make behavior better in the latest version. mav->haggai: The fact that the patch can not be integrated as it is does not reduce the value of the investigations. :) As for the race condition, actually the read only mode of OOo is a designed race condition itself, since the document file state can be changed while having the document open. The only possible problem while using the attributes, as you have correctly pointed, is that the document gets readonly state while being opened/edited. The OOo implementation should be ready for it and handle it accordingly - either the file would be opened readonly ( if it happens between the calls as you have mentioned ) or an error would be shown on saving ( if it happens during the editing ). Thus I still tend to think that the correct approach would be to fix the attributes handling. [I've been looking at this problem with haggai.] diocles->mav: Fixing the attribute handling in osl_getFileStatus is /just about/ possible, but ugly... I really do not think we want to reimplement the OS's POSIX ACL checks in userspace - it is likely to be complicated and prone to error, and will still not take account of such things as NFSv4 ACLs or SELinux/Apparmor permissions. There is a POSIX access(2) function (does this in fact respect ACLs? probably not), but there is a comment above set_file_access_rights() that there is a potential performance problem involving symlinks when using it. The code path which attempts to open the stream as writable and handles the error condition is vastly more reliable. How would this break exactly when file locking is turned off? mav->diocles: When the system file locking is off the document stream is opened readonly. We could of course try to open it read-write, but on some systems it is not possible to open a document in read-write mode without system file locking being used. I understand your concerns, but sorry, the file status handling should work as expected. If it can not work so in general, this API just has no sense. Second, I would not like to introduce a special solution for the case when file-system lock is active. There are already enough differences between these cases in office behavior, and any change that lets file-system lock and OOo lock scenarios behave differently should have a good reason. Ok, it is probably indeed a good enough reason to introduce the difference between system file locking implementation and OOo file locking implementation. Actually, the mode when system file locking is off is used mostly in cases when it is not reliable. In this case a non-perfect implementation of ACL handling would be acceptable. I shall change the implementation in the way that the file is opened directly for editing ( without file status evaluation ) in case system file locking is active. Mikhail, is this issue still planned for 3.2? Regards, Kirill Palagin. Fixed in fwk116. Now the office tries to open the file for editing always. There is still a problem with this solution. If the problematic file is locked by another thirdparty application using the system locking, the office is not able to detect it. The reason for this is that if the file can not be opened for editing, the office still has to use osl_getFileStatus() to detect whether the file is readonly, and only if it is not readonly the office treats it as a locked one. So for this scenario we still have to improve osl_getFileStatus(), although this scenario looks to be much less problematic than the original one described in this issue. mav->tm: Please test the issue. To test it you could use the following steps on solaris ( the cws has unxsoli4.pro build, so it could be done on Intel Solaris machine ) - create a document on disk that supports ACL ( for example "tausch" ) - make "chmod 444 <path to the document>" - "setfacl -r -m group:<group name>:rw- <path to the document>" - let another user from <group name> group open the document using the CWS installation => the document should be editable. Just to clear the problem described in my comment from Sep 2. The mentioned document will not be recognized as locked one in the described in the comment scenario, instead it will be handled as a read-only document. *** Issue 101922 has been marked as a duplicate of this issue. *** checked and verified in cws fwk116 -> OK ! |