Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:15046
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
To: "'Rob Richards'" <rrichards@ctindustries.net>
Cc: "'Christian Stocker'" <chregu@bitflux.ch>,
	<internals@lists.php.net>
Date: Thu, 17 Feb 2005 13:32:11 +0100
MIME-Version: 1.0
Content-Type: text/plain;
	charset="us-ascii"
Content-Transfer-Encoding: 7bit
thread-index: AcUU2yH0eH1zArHQTLqrpEyDxQRvZwADdc7A
In-Reply-To: <42147223.5090601@ctindustries.net>
Message-ID: <fd2loqwtrkrn02z.170220051332@127.0.0.1>
Subject: RE: [PHP-DEV] Re: [patch] DOM canonize method
From: D.Kingma@jool.nl ("David Kingma | jool.nl")
References: <42147223.5090601@ctindustries.net>

Hi Rob,

Tank you for you extensive reply. 

> -----Original Message-----
> A few comments from eyeballing the patch:
> 
> - Depending upon how much c14n functionality there is to be, 
> this might be better off as a standalone extension. This way 
> it wouldn't be dependant upon DOM (especially if PHP is built 
> without it) and allow it to work with simplexml alone. Take 
> xsl for example (although it requires dom to output a dom 
> document, it allows processing via simplexml input - as 
> simplexml has no concept of a document i allow any simplexml 
> element to be used that has a document and just use the 
> document from that node.

This is about all the C14n functionality there is. I agree with you on it
being dependant upon DOM. Don't you think it's to little functionality for
an extension on his own? I thought it would fit nicely into the DOM
extension. And this way, it's bundled by default :).

> - Looking at the patch, you may want to change the optional 
> node argument to an array so that multiple nodes can be used 
> for the visibility test
> 
> - node needs to be initialized to NULL other wise if none is 
> passed it most likely will crash. that being said, if none is 
> passed in you should probably test and use xmlC14NDocSaveTo 
> as it will run much faster as no callback would need to be run.
> 
> - no need to dupe the xml output buffer content and then dupe 
> it again when creating a zval. would dupe the output buffer 
> straight into the zval.
> 
> - str_inclusive_ns parameter is st as a string. This however 
> needs to allow to a list of strings since 
> inclusive_ns_prefixes is a xmlChar ** which will be iterated 
> to get all the prefixes.
> 
> Thats pretty much what jumped out at me as I mentioned I have 
> only looked at the patch and havent played with it.

Thanks, I will look into those points.

> If you are really looking at implementing the xml-security 
> specs, you might want to think about just creating an 
> extension from the xmlsec library. Developng  xmlsec is very 
> complicated (just check out the libxml mailing list as this 
> has been brought up before and people are always steered to 
> the xmlsec library). I had started a while back working on an 
> extension for this, but other than some early layout code I 
> haven't found the time to work on it, so before diving in 
> trying to write it from scratch you might want to check out 
> that library.
> 
> http://www.aleksey.com/xmlsec/

I'm aware of the xmlsec lib from aleksey (that's where the C14n functions
originally came from) and I have no intentions to re-invent the wheel,
especially not when it's as complicated as the xmlsec lib :). I wrote the
c14n patch as a practise to get familiar with libxml and php extensions (and
C), but it also allows people to use it in PHP to start hacking some
signature scripts (together with openSSL extension ofcourse).

Could you send me your 'early layout code'?

Regards,

David