[xmlsec] core methods for write of <X509SubjectName/> and <X509IssuerSerial/>

[Aleksey Sanin]

Hi, Roumen!

I have looked at your new patch and I have few comments:
    0) It seems that xmlSecOpenSSLKeyDataX509XmlWrite() function
    now writes subject, serial or full certificate only for the first 
certificate
    in the xmlSecKey. All other certificates are written "in-full".
    This seems wrong to me. Yo don't know which certificate will be the
    "first" one. I am not sure I understand why you don't want to do the
    same for all certs.
    I thought that the plan was:  
          - Read X509Data node and create a bits mask of its children
          (cert, subject, serial, ski, crl).
          - If mask is 0 (no children) then set cert and crl bits to 
simulate
          current behaiviour (write certs and crls in empty X509Data node).
          - Remove X509Data node content.
          - Walk thru the list of certificates and write cert and/or 
subject and/or
          serial and/or ski according to bits mask.
          - If crls bit is set walk thru the list of crls and write them 
out.
    This seems more natural to me than "special case" the first cert.

    1) I don't like the way you implemented the "empty" check in *Read() 
functions.
    IMHO, this is a bad coding style to repeat the same code again and  
again.
    Probably a small internal static function
        int xmlSecOpenSSLX509IsEmpty(xmlChar*)
    would be better :)
    Also I am not sure I understand why you put "XXX" comments around 
it. Seems
    useless to me.

    2) You are using the figure brackets to mark block of code all the time
    (I meant the "write Issuer Name" block in the example bellow):
     
+   if(cur == NULL) {
+       cur = xmlSecAddChild(node, xmlSecNodeX509IssuerSerial, 
xmlSecDSigNs);
+       ....
+    }
+
+    { /*write Issuer Name*/
+       for(node_in = xmlSecGetNextElementNode(cur->children);
+       ....
+    }

    Please don't do this. It makes code difficult to read.

    3) In xmlSecOpenSSLX509NameWrite() function I wonder if there is a 
way to
    print name to a buffer, not memory BIO. Mallocs might be expensive 
:( But I guess
    the answer is "no".

    4) Which OpenSSL version do you use? I wonder if this new code works 
with OpenSSL 0.9.6.


Aleksey


-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://www.aleksey.com/pipermail/xmlsec/attachments/20030723/97d87c1e/attachment.htm