Re: [CalendarServer-changes] [1064] CalendarServer/trunk/lib-patches/Twisted/twisted.web2.dav.element. base.patch
What exactly is the issue with XML attributes? If they aren't in the allowed list, we should raise an error or something, no? Do we have to allow arbitrary unknown attributes to pass through? -wsv On Jan 18, 2007, at 1:30 PM, source_changes@macosforge.org wrote:
Revision 1064 Author cdaboo@apple.com Date 2007-01-18 13:30:27 -0800 (Thu, 18 Jan 2007) Log Message
Fix for issue with XML attributes on properties being lost. Modified Paths
CalendarServer/trunk/lib-patches/Twisted/ twisted.web2.dav.element.base.patch Diff
Modified: CalendarServer/trunk/lib-patches/Twisted/ twisted.web2.dav.element.base.patch (1063 => 1064)
--- CalendarServer/trunk/lib-patches/Twisted/ twisted.web2.dav.element.base.patch 2007-01-18 21:15:52 UTC (rev 1063) +++ CalendarServer/trunk/lib-patches/Twisted/ twisted.web2.dav.element.base.patch 2007-01-18 21:30:27 UTC (rev 1064) @@ -2,7 +2,36 @@ =================================================================== --- twisted/web2/dav/element/base.py (revision 18545) +++ twisted/web2/dav/element/base.py (working copy) -@@ -190,14 +190,93 @@ +@@ -145,21 +145,20 @@ + + if self.allowed_attributes: + for name in attributes: +- if name in self.allowed_attributes: +- my_attributes[name] = attributes[name] +- else: +- log.msg("Attribute %s is unexpected and therefore ignored in %s element" +- % (name, self.sname())) ++ if name not in self.allowed_attributes: ++ log.msg("Attribute %s is unexpected in %s element" % (name, self.sname())) ++ my_attributes[name] = attributes[name] + + for name, required in self.allowed_attributes.items(): + if required and name not in my_attributes: + raise ValueError("Attribute %s is required in %s element" + % (name, self.sname())) + +- elif not isinstance(self, WebDAVUnknownElement): +- if attributes: +- log.msg("Attributes %s are unexpected and therefore ignored in %s element" ++ else: ++ if not isinstance(self, WebDAVUnknownElement) and attributes: ++ log.msg("Attributes %s are unexpected in %s element" + % (attributes.keys(), self.sname())) ++ my_attributes.update(attributes) + + self.attributes = my_attributes + +@@ -190,14 +189,93 @@ return child in self.children
def writeXML(self, output): @@ -100,7 +129,7 @@
def element(self, document): element = document.createElementNS(self.namespace, self.name) -@@ -324,6 +403,22 @@ +@@ -324,6 +402,22 @@ log.err("Invalid PCDATA: %r" % (self.data,)) raise
_______________________________________________ calendarserver-changes mailing list calendarserver-changes@lists.macosforge.org http://lists.macosforge.org/mailman/listinfo/calendarserver-changes
Hi Wilfredo, --On January 19, 2007 1:38:06 PM -0800 Wilfredo Sánchez Vega <wsanchez@wsanchez.net> wrote:
What exactly is the issue with XML attributes? If they aren't in the allowed list, we should raise an error or something, no? Do we have to allow arbitrary unknown attributes to pass through?
Pretty much yes. The specific case here was an xml:lang attribute on DAV:displayname which is perfectly fine. Arguably we could perhaps have made xml:lang an allowed_attribute (optional) on WebDAVTextElement, but maybe it could appear on some other type of element. BTW xml:space is another attribute that can appear on text elements, though 2518bis says that one MUST be ignored in terms of actual text processing. The point is there could be other such xml:... attributes that are significant that we should accept (be liberal in what you accept - with the proviso that it does no harm). The fact is WebDAV is very lax in its use of "strict" XML syntax. It really ought to explicitly specify which elements can have an xml:lang (as required by W3C) but it does not. 2518 section14 makes it clear that servers must ignore unknown elements and I take that to also mean unknown attributes. In fact if we take that literally we also ought to relax the allowed_children behavior to allow unknown elements in. BTW The XML validation stuff was one area that showed up as needing performance improvement - there are a lot of calls to WebDAVElement.__init__ when building up a large PROPFIND or REPORT response. Arguably we could find a way to turn off validation for XML generated by the server on the grounds that we know what we are doing (though keep it for debugging) - but we definitely want validation of incoming XML from the client. -- Cyrus Daboo
participants (2)
-
Cyrus Daboo
-
Wilfredo Sánchez Vega