Are you still thinking we should also scan the document for duplicate IDs? --Sean
On 12/21/11 7:14 AM, Chad La Joie wrote: > So, last night I spent some time digging in to the new code and > discussing with some things with Scott. Here's where we ended up. > > The IdResolver is not designed in a manner that would allow alternative > implementations to be used. Though, given that it is only used by the > ResourceResolver implementations, those could be replaced by the user of > the library > > The new code does use the Document.getElementById method to look up IDs > in the general case. This may be overridden by explicitly placing > ID-to-Element mappings into the IdResolver, but since doing this is > neither documented nor shown in the examples it is fairly safe to assume > this isn't happening out in the wild. > > Given the above, this code does not guard against any issues that may > arise from duplicate IDs. > > In the case of signatures, there seems to be nothing that a library can > do in order to ensure that the "right thing" is covered by the signature > since the "right thing" is use-case specific. > > In the case of encryption, there is no way for the application to know > whether the dereferenced encrypted encryption key is what the document > author actually intended. If it's not, however, the worst that would > happen is that encrypted data would not decrypt. > > Given this, I believe the following things should be done: > - The use of Document.getElementById and the potential issue caused by > the lack of a strong guarantee on ID uniqueness should be documented. > - The documentation for XMLSignature and all the examples should be > updated to make it clear that the user of the library needs to check > that what they thought was covered by the signature actually was. > - Attempting to update the examples in this way will make it clear that > better APIs need to be provided to expose the content indicated by > <Reference>s. > > In addition, given the manner in which the IdResolver currently works, I > believe it could be done away with entirely. The > IdResolver.registerElementById method could simply be replaced by the > appropriate Element.setIdAttribute* method. Doing so would remove a > potentially large in-memory data structure as well as various > synchronization points that currently prevent concurrent ID resolutions. >
