Re: names starting with '@' are not reserved
yuppie wrote:
In Zope 3 the NameChooser makes sure you can't use content IDs starting with '+' or '@'.
Zope 2 doesn't allow '+' in content IDs (actually the error message says the ID contains characters illegal in URLs), but you can use content IDs like '@@edit.html'. If the lookup order is changed as proposed (http://codespeak.net/pipermail/z3-five/2006q1/001186.html) this allows to override views with content objects.
I guess this is a bug
I wouldn't say that. It's just that something Five added (Zope 3 view lookup) doesn't work in all cases of legal Zope 2 object names. That's Five's problem, not Zope 2's (strictly speaking).
and should be fixed in Zope 2.8, 2.9 and trunk.
We'd be changing Zope 2's behaviour (not fixing a Five bug), I would therefore vote for making this change on the Zope 2 trunk only. CCing zope-dev as they will probably have an opinion there as well. Philipp
yuppie wrote:
Zope 2 doesn't allow '+' in content IDs (actually the error message says the ID contains characters illegal in URLs), but you can use content IDs like '@@edit.html'. If the lookup order is changed as proposed (http://codespeak.net/pipermail/z3-five/2006q1/001186.html) this allows to override views with content objects.
I guess this is a bug
I wouldn't say that. It's just that something Five added (Zope 3 view lookup) doesn't work in all cases of legal Zope 2 object names. That's Five's problem, not Zope 2's (strictly speaking).
and should be fixed in Zope 2.8, 2.9 and trunk.
We'd be changing Zope 2's behaviour (not fixing a Five bug), I would therefore vote for making this change on the Zope 2 trunk only.
CCing zope-dev as they will probably have an opinion there as well.
I'd be apt to not further restrict the set of identifiers an OFS content object can begin with as long as we can get away with it. If a user happens to enter one that starts with a "view character", I suspect a developer could just choose to not allow this in at the application level if he cared (e.g. if he were using Five). It might be reasonable to add this to CMF instead of Zope, but IMO, it's a bad idea to put theis particular restriction in either place, as it will almost certainly break somebody. - C
Chris McDonough wrote:
and should be fixed in Zope 2.8, 2.9 and trunk.
We'd be changing Zope 2's behaviour (not fixing a Five bug), I would therefore vote for making this change on the Zope 2 trunk only.
I'd be apt to not further restrict the set of identifiers an OFS content object can begin with as long as we can get away with it. If a user happens to enter one that starts with a "view character", I suspect a developer could just choose to not allow this in at the application level if he cared (e.g. if he were using Five). It might be reasonable to add this to CMF instead of Zope, but IMO, it's a bad idea to put theis particular restriction in either place, as it will almost certainly break somebody.
Yup. That's why we should do this change on the trunk IF at all. Note that even in Zope 3 the suggested behaviour is only the default and can be changed. Custom name chooser adapters may decide whether names are good or not based on whatever rule. Therefore, if we introduce this restriction, it should be pluggable like in Zope 3. Here's what we could do: We factor the name validation part in ObjectManager (which is _checkId) out to a namechooser adapter. Five already has one in Five.browser.adding. Then, we can also provide an optional namechooser adapter that enforces the additional restrictions. People could decide to use this for their folder implementation, e.g. the CMF might want to do it for their folders. Philipp ---------------------------------------------------------------- This message was sent using IMP, the Internet Messaging Program.
On Mon, 2006-03-13 at 02:46 +0100, Philipp von Weitershausen wrote:
Here's what we could do: We factor the name validation part in ObjectManager (which is _checkId) out to a namechooser adapter. Five already has one in Five.browser.adding. Then, we can also provide an optional namechooser adapter that enforces the additional restrictions. People could decide to use this for their folder implementation, e.g. the CMF might want to do it for their folders.
That sounds about right to me. - C
Hi! Philipp von Weitershausen wrote:
Chris McDonough wrote:
and should be fixed in Zope 2.8, 2.9 and trunk. We'd be changing Zope 2's behaviour (not fixing a Five bug), I would therefore vote for making this change on the Zope 2 trunk only. I'd be apt to not further restrict the set of identifiers an OFS content object can begin with as long as we can get away with it. If a user happens to enter one that starts with a "view character", I suspect a developer could just choose to not allow this in at the application level if he cared (e.g. if he were using Five). It might be reasonable to add this to CMF instead of Zope, but IMO, it's a bad idea to put theis particular restriction in either place, as it will almost certainly break somebody.
Yup. That's why we should do this change on the trunk IF at all.
I doubt this will break a significant amount of code. The restriction was removed 5 months ago and AFAICS it was removed to allow email addresses as IDs. That use case will not be broken if we disallow again IDs starting with '@'.
Note that even in Zope 3 the suggested behaviour is only the default and can be changed. Custom name chooser adapters may decide whether names are good or not based on whatever rule. Therefore, if we introduce this restriction, it should be pluggable like in Zope 3.
If we ignore for a moment the fact that for the last 5 months the hole was open reserving '@' for views doesn't introduce any new restriction in Zope 2. '@' was always disallowed. Removing that restriction for the first character was a mistake. I just want to revert that part of the change until we have a pluggable solution.
Here's what we could do: We factor the name validation part in ObjectManager (which is _checkId) out to a namechooser adapter. Five already has one in Five.browser.adding.
This is obviously the Right Thing to do in the long term. But doesn't help us resolving the bug in Zope 2.8 and 2.9.
Then, we can also provide an optional namechooser adapter that enforces the additional restrictions. People could decide to use this for their folder implementation, e.g. the CMF might want to do it for their folders.
We already have @@manage_interfaces and Zope 2 containers will have more Zope 3 style views in the future. So I think the default in OFS should be the same as the default in Zope 3. Cheers, Yuppie
I doubt this will break a significant amount of code. The restriction was removed 5 months ago and AFAICS it was removed to allow email addresses as IDs. That use case will not be broken if we disallow again IDs starting with '@'.
It seems that you can reasonably easily apply the "@" restriction in your own app code under 2.8 and 2.9 until we get around to doing the "right" thing by making this pluggable via the namechooser thingy, no? I dunno. The current set of restrictions is too restrictive for "real world" use when you use Zope heavily as a "fileserver", say via DAV. I wouldn't treat the existing or older restrictions as "gospel" as a result. I had actually been meaning to get around to unrestricting the set of identifiers in the trunk. Here's my current monkeypatch to Zope to unrestrict a good number of characters: def patch_objectmanager_badid(): """ Causes Zope to be less restrictive in the set of characters it accepts as valid within object identifiers. Added as acceptable: []*'!:@&#=+$ """ import re acceptable = r'[^a-zA-Z0-9-_~,.$\(\)\[\]\*\'\!\:\@\&\#\=\+\$ ]' bad_id = re.compile(acceptable).search import OFS.ObjectManager OFS.ObjectManager.bad_id = bad_id The projects that use this patch have been in use for several years; they predate Five. I of course don't mind continuing to do this, but I'd hate to have to change it temporarily (to fix this bug which actually isn't a bug for me because I don't use Five for these projects) and then change it again when we do the pluggable thing. I suppose I wouldn't care if the change was isolated to the "bad_id" regex, given that I replace it anyway. - C
Hi Chris! Chris McDonough wrote:
I doubt this will break a significant amount of code. The restriction was removed 5 months ago and AFAICS it was removed to allow email addresses as IDs. That use case will not be broken if we disallow again IDs starting with '@'.
It seems that you can reasonably easily apply the "@" restriction in your own app code under 2.8 and 2.9 until we get around to doing the "right" thing by making this pluggable via the namechooser thingy, no? I dunno.
The current set of restrictions is too restrictive for "real world" use when you use Zope heavily as a "fileserver", say via DAV. I wouldn't treat the existing or older restrictions as "gospel" as a result. I had actually been meaning to get around to unrestricting the set of identifiers in the trunk.
I'd like to see this improved. But I think we need a solution that works by default with Zope 3 style views.
Here's my current monkeypatch to Zope to unrestrict a good number of characters:
[..]
The projects that use this patch have been in use for several years; they predate Five. I of course don't mind continuing to do this, but I'd hate to have to change it temporarily (to fix this bug which actually isn't a bug for me because I don't use Five for these projects) and then change it again when we do the pluggable thing.
I suppose I wouldn't care if the change was isolated to the "bad_id" regex, given that I replace it anyway.
I'm not concerned about my own app code. I know the problem and how to fix it. And I'm not concerned about people like you who monkeypatch that code. You know that monkeypatching is always on your own risk and you know how to modify your monkey patch even if more code is changed than 'bad_id'. I'm concerned about the people we encourage to use Five technology. Views are a major feature of Five. Should we warn people not to use views? Or instruct them how to patch Zope 2 to protect views against being masked by content IDs? Cheers, Yuppie
On Mon, Mar 13, 2006 at 07:06:28PM +0100, yuppie wrote:
I'm concerned about the people we encourage to use Five technology. Views are a major feature of Five. Should we warn people not to use views? Or instruct them how to patch Zope 2 to protect views against being masked by content IDs?
Or just document a warning that content whose ids begin with @@ can mask views? I'm wondering if this is a case of "Doctor, it hurts when I do this..." -- Paul Winkler http://www.slinkp.com
Hi Paul! Paul Winkler wrote:
On Mon, Mar 13, 2006 at 07:06:28PM +0100, yuppie wrote:
I'm concerned about the people we encourage to use Five technology. Views are a major feature of Five. Should we warn people not to use views? Or instruct them how to patch Zope 2 to protect views against being masked by content IDs?
Or just document a warning that content whose ids begin with @@ can mask views?
I'm wondering if this is a case of "Doctor, it hurts when I do this..."
It's quit common that normal users of Zope applications are allowed to add content. You can educate programmers, but you can't solve a problem like this by educating (sometimes untrusted) users. They can easily screw up a Zope app by overriding important views. And if they can do it some (untrusted) users will do it. Cheers, Yuppie
On Mar 13, 2006, at 1:06 PM, yuppie wrote:
I'm not concerned about my own app code. I know the problem and how to fix it.
And I'm not concerned about people like you who monkeypatch that code. You know that monkeypatching is always on your own risk and you know how to modify your monkey patch even if more code is changed than 'bad_id'.
I'm concerned about the people we encourage to use Five technology. Views are a major feature of Five. Should we warn people not to use views? Or instruct them how to patch Zope 2 to protect views against being masked by content IDs?
IMO, we should fix it "right" and live with the status quo until we do (which is that content ids can shadow views). I don't think it's worth it to hack it in the meantime. It just doesn't seem like that much of an emergency, IMO. - C
On Mar 13, 2006, at 1:22 PM, Chris McDonough wrote:
On Mar 13, 2006, at 1:06 PM, yuppie wrote:
I'm not concerned about my own app code. I know the problem and how to fix it.
And I'm not concerned about people like you who monkeypatch that code. You know that monkeypatching is always on your own risk and you know how to modify your monkey patch even if more code is changed than 'bad_id'.
I'm concerned about the people we encourage to use Five technology. Views are a major feature of Five. Should we warn people not to use views? Or instruct them how to patch Zope 2 to protect views against being masked by content IDs?
IMO, we should fix it "right" and live with the status quo until we do (which is that content ids can shadow views). I don't think it's worth it to hack it in the meantime. It just doesn't seem like that much of an emergency, IMO.
Also, FWIW, it just occurs to me that even though I do use Five, I've never generated a "@@" URL. It appears purely optional to use the "@@" syntax in the URL to call a view. Most of the examples I've seen out there don't use it either. - C
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Chris McDonough wrote:
On Mar 13, 2006, at 1:22 PM, Chris McDonough wrote:
On Mar 13, 2006, at 1:06 PM, yuppie wrote:
I'm not concerned about my own app code. I know the problem and how to fix it.
And I'm not concerned about people like you who monkeypatch that code. You know that monkeypatching is always on your own risk and you know how to modify your monkey patch even if more code is changed than 'bad_id'.
I'm concerned about the people we encourage to use Five technology. Views are a major feature of Five. Should we warn people not to use views? Or instruct them how to patch Zope 2 to protect views against being masked by content IDs?
IMO, we should fix it "right" and live with the status quo until we do (which is that content ids can shadow views). I don't think it's worth it to hack it in the meantime. It just doesn't seem like that much of an emergency, IMO.
Also, FWIW, it just occurs to me that even though I do use Five, I've never generated a "@@" URL. It appears purely optional to use the "@@" syntax in the URL to call a view. Most of the examples I've seen out there don't use it either.
Views for containers which want to disambiguate view lookup from item traversal should use '@@view.html'. Non-containers don't have the ambiguity, and hence don't need the hint. Tres. - -- =================================================================== Tres Seaver +1 202-558-7113 tseaver@palladion.com Palladion Software "Excellence by Design" http://palladion.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFEFhV/+gerLs4ltQ4RAhF6AKDcCXyazrcccA5BUlyLvZzXBxSiIACfQxK9 DVxm3NYHFNh21CDB23pae8g= =nyId -----END PGP SIGNATURE-----
Chris McDonough wrote at 2006-3-13 10:21 -0500:
... silly id restrictions ... Here's my current monkeypatch to Zope to unrestrict a good number of characters:
def patch_objectmanager_badid(): """ Causes Zope to be less restrictive in the set of characters it accepts as valid within object identifiers.
Added as acceptable: []*'!:@&#=+$ """
import re acceptable = r'[^a-zA-Z0-9-_~,.$\(\)\[\]\*\'\!\:\@\&\#\=\+\$ ]' bad_id = re.compile(acceptable).search import OFS.ObjectManager OFS.ObjectManager.bad_id = bad_id
The projects that use this patch have been in use for several years; they predate Five. I of course don't mind continuing to do this, but I'd hate to have to change it temporarily (to fix this bug which actually isn't a bug for me because I don't use Five for these projects) and then change it again when we do the pluggable thing.
+1 Looks as if we had very similar project requirements... -- Dieter
Dieter Maurer wrote:
Chris McDonough wrote at 2006-3-13 10:21 -0500:
... silly id restrictions ... Here's my current monkeypatch to Zope to unrestrict a good number of characters:
def patch_objectmanager_badid(): """ Causes Zope to be less restrictive in the set of characters it accepts as valid within object identifiers.
Added as acceptable: []*'!:@&#=+$ """
import re acceptable = r'[^a-zA-Z0-9-_~,.$\(\)\[\]\*\'\!\:\@\&\#\=\+\$ ]' bad_id = re.compile(acceptable).search import OFS.ObjectManager OFS.ObjectManager.bad_id = bad_id
The projects that use this patch have been in use for several years; they predate Five. I of course don't mind continuing to do this, but I'd hate to have to change it temporarily (to fix this bug which actually isn't a bug for me because I don't use Five for these projects) and then change it again when we do the pluggable thing.
+1
Looks as if we had very similar project requirements...
Chris's and Dieter's requirements seem to even more confirm my proposal that we should propertly factor this out to a name chooser adapter that everyone can configure for themselves. Then this discussion what ObjectManager should do or not do will become irrelevant because it won't do anything anymore :). This is actually how Zope 3 containers work. They perform no name checks *at all*. It's the application (in particular, the adding view) that does it. Yuppie's concern are Zope versions 2.8 and 2.9. I say that we'd just have to live with the fact that objects can shadow views there. Applications like the CMF can make sure that they don't on an application-level, as Chris suggests, preferrably through a name chooser adapter. Philipp ---------------------------------------------------------------- This message was sent using IMP, the Internet Messaging Program.
Hi Philipp! Philipp von Weitershausen wrote:
Dieter Maurer wrote:
Chris McDonough wrote at 2006-3-13 10:21 -0500:
... silly id restrictions ... Here's my current monkeypatch to Zope to unrestrict a good number of characters:
def patch_objectmanager_badid(): """ Causes Zope to be less restrictive in the set of characters it accepts as valid within object identifiers.
Added as acceptable: []*'!:@&#=+$ """
import re acceptable = r'[^a-zA-Z0-9-_~,.$\(\)\[\]\*\'\!\:\@\&\#\=\+\$ ]' bad_id = re.compile(acceptable).search import OFS.ObjectManager OFS.ObjectManager.bad_id = bad_id
The projects that use this patch have been in use for several years; they predate Five. I of course don't mind continuing to do this, but I'd hate to have to change it temporarily (to fix this bug which actually isn't a bug for me because I don't use Five for these projects) and then change it again when we do the pluggable thing. +1
Looks as if we had very similar project requirements...
Chris's and Dieter's requirements seem to even more confirm my proposal that we should propertly factor this out to a name chooser adapter that everyone can configure for themselves. Then this discussion what ObjectManager should do or not do will become irrelevant because it won't do anything anymore :). This is actually how Zope 3 containers work. They perform no name checks *at all*. It's the application (in particular, the adding view) that does it.
Zope 2's ObjectManager class is not as abstract as Zope 3 containers are. It provides a lot of folder specific behavior. I guess it would be better to subclass ObjectManager from a generic container class than trying to move all non-generic code in subclasses of ObjectManager.
Yuppie's concern are Zope versions 2.8 and 2.9.
Depends on the proposed solution for Zope 2.10. If you want to make a distinction between ObjectManager and Folder I can live with it. But Zope 2 folders should (by default) perform the same name checks as Zope 3 folders. And they don't allow names starting with '@'.
I say that we'd just have to live with the fact that objects can shadow views there. Applications like the CMF can make sure that they don't on an application-level, as Chris suggests, preferrably through a name chooser adapter.
I try to restate the problem: - It is quite common that normal users are allowed to add objects in Zope applications. - If views are shadowed by objects this can seriously break the app. - Good software makes sure normal users can't break the app. Zope 2's checkValidId makes sure this doesn't happen with Zope 2 folder methods, Zope 3's NameChooser makes sure this doesn't happen with Zope 3 folder views. Even the bad_id-patch described above doesn't allow to override folder methods. Making the name chooser configurable doesn't release us from the need to provide a good default name chooser. I still believe this should be fixed as I proposed, but given the resistance I give up my attempt to get this fixed. This is now http://www.zope.org/Collectors/Zope/2048 and I hope someone else will fix it. Cheers, Yuppie
yuppie wrote at 2006-3-15 11:23 +0100:
... Zope 2's checkValidId makes sure this doesn't happen with Zope 2 folder methods, Zope 3's NameChooser makes sure this doesn't happen with Zope 3 folder views. Even the bad_id-patch described above doesn't allow to override folder methods.
Maybe, the "checkValidId" should refuse to add an object with an id that hides a view declared for this folder and not reject any id that might (potentially) hide a view because it starts with "@" or "+"... This would prevent the security concerns you seem to have and allows for most ids to be accepted... -- Dieter
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Dieter Maurer wrote:
yuppie wrote at 2006-3-15 11:23 +0100:
... Zope 2's checkValidId makes sure this doesn't happen with Zope 2 folder methods, Zope 3's NameChooser makes sure this doesn't happen with Zope 3 folder views. Even the bad_id-patch described above doesn't allow to override folder methods.
Maybe, the "checkValidId" should refuse to add an object with an id that hides a view declared for this folder and not reject any id that might (potentially) hide a view because it starts with "@" or "+"...
This would prevent the security concerns you seem to have and allows for most ids to be accepted...
Such objects would still suffer from potential future namespace clashes with views not yet declared, or even not yet appropriate to the object in its current state (e.g, should it acquire a new marker interface, its set of views would be larger). I would think that the reasonable thing to do here is to make the "id validation" policy pluggable (e.g., via an adapter), so that users with different needs can supply appropriate policies. The question then becomes which policy should be the default. Given that such IDs are only recently possible in Zope, I would say using a more restrictive policy by default would be sensible. Tres. - -- =================================================================== Tres Seaver +1 202-558-7113 tseaver@palladion.com Palladion Software "Excellence by Design" http://palladion.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFEGLUt+gerLs4ltQ4RAiHvAJ9MYRbR7xARubp/yX6W22tABURpxwCff4Ls /Ru0OVluMzODwSge3eAhf7U= =f/Iz -----END PGP SIGNATURE-----
Philipp von Weitershausen wrote:
yuppie wrote:
In Zope 3 the NameChooser makes sure you can't use content IDs starting with '+' or '@'.
Zope 2 doesn't allow '+' in content IDs (actually the error message says the ID contains characters illegal in URLs), but you can use content IDs like '@@edit.html'. If the lookup order is changed as proposed (http://codespeak.net/pipermail/z3-five/2006q1/001186.html) this allows to override views with content objects.
I guess this is a bug
I wouldn't say that. It's just that something Five added (Zope 3 view lookup) doesn't work in all cases of legal Zope 2 object names. That's Five's problem, not Zope 2's (strictly speaking).
I don't understand that differentiation. Five is part of Zope 2, so each Five issue is also a Zope 2 issue.
and should be fixed in Zope 2.8, 2.9 and trunk.
We'd be changing Zope 2's behaviour (not fixing a Five bug), I would therefore vote for making this change on the Zope 2 trunk only.
Fixing a bug sometimes requires to change the behavior. Cheers, Yuppie
yuppie wrote at 2006-3-13 10:09 +0100:
... I don't understand that differentiation. Five is part of Zope 2, so each Five issue is also a Zope 2 issue.
But you can fix this issue either in "Five" or in "ObjectManager". You propose a change in "ObjectManager" which affects not Five use of Zope2. -- Dieter
participants (6)
-
Chris McDonough -
Dieter Maurer -
Paul Winkler -
Philipp von Weitershausen -
Tres Seaver -
yuppie