[SR-Users] Does xcap server of kamailio support partial update of xcap document?
Daniel-Constantin Mierla
miconda at gmail.com
Tue Sep 13 19:05:43 CEST 2011
Hi Laura,
thanks for the patch. I applied to master branch for now a slightly
different version of it, by adding a new function that just checks the
validity of xml and it is used for db insert/select of xcap documents.
Cheers,
Daniel
On 9/13/11 5:33 PM, laura testi wrote:
> Hi Daniel,
> we found further bugs in the file xcap_server.c (lastest version on
> master branch with the patches).
> The main problems are following:
> - xcap server insert the xml doc into DB without any validation of
> the xml doc for the operation
> PUT, which can cause core dump when the DELETE operation is
> performed if the XML doc
> is invalid in DB
> - the kamailio crash when DELETE operation is performed if in the DB
> there is no referenced
> xml doc
>
>
> We have done further patches on the lastest xcap_server.c from the
> master branch.
>
> Here are the diff:
>
> # diff -u xcap_server.c.orig xcap_server.c.new
> --- xcap_server.c.orig 2011-09-13 11:52:19.000000000 +0200
> +++ xcap_server.c.new 2011-09-13 15:12:54.000000000 +0200
> @@ -30,6 +30,8 @@
> #include<unistd.h>
> #include<fcntl.h>
> #include<time.h>
> +#include<libxml/xpath.h>
> +#include<libxml/xpathInternals.h>
>
> #include "../../lib/srdb1/db.h"
> #include "../../pt.h"
> @@ -334,7 +336,14 @@
> db_key_t qcols[9];
> db_val_t qvals[9];
> int ncols = 0;
> -
> + xmlDocPtr docxml = NULL;
> +
> + docxml = xmlParseMemory(doc->s, doc->len);
> + if(docxml == NULL)
> + {
> + LM_ERR("Error inserting Invalid Xml Document into database \n");
> + goto error;
> + }
>
> /* insert in xcap table*/
> qcols[ncols] =&str_username_col;
> @@ -400,6 +409,7 @@
>
> return 0;
> error:
> +if(docxml!=NULL) xmlFreeDoc(docxml);
> return -1;
> }
>
> @@ -603,6 +613,7 @@
> int nrcols = 0;
> db1_res_t* db_res = NULL;
> str s;
> + xmlDocPtr docxml = NULL;
>
> /* returned cols from table xcap*/
> rcols[nrcols] =&str_doc_col;
> @@ -680,7 +691,15 @@
> memcpy(doc->s, s.s, s.len);
> doc->s[doc->len] = '\0';
>
> + docxml = xmlParseMemory(doc->s, doc->len);
> + if(docxml == NULL)
> + {
> + LM_ERR("Invalid XmlDocument in database \n");
> + goto error;
> + }
> +
> xcaps_dbf.free_result(xcaps_db, db_res);
> + if(docxml!=NULL) xmlFreeDoc(docxml);
> return 0;
>
> notfound:
> @@ -690,6 +709,7 @@
> error:
> if(db_res!=NULL)
> xcaps_dbf.free_result(xcaps_db, db_res);
> + if(docxml!=NULL) xmlFreeDoc(docxml);
> return -1;
> }
>
> @@ -970,7 +990,7 @@
> str uri;
> str path;
> xcap_uri_t xuri;
> - str body;
> + str body = {0, 0};
> str etag_hdr;
> str etag;
> str tbuf;
> @@ -1041,7 +1061,7 @@
> &xcaps_str_empty,&xcaps_str_empty);
> } else {
> /* delete element */
> - if(xcaps_get_db_doc(&turi.user,&turi.host,&xuri,&tbuf)<0)
> + if(xcaps_get_db_doc(&turi.user,&turi.host,&xuri,&tbuf)!=0)
> {
> LM_ERR("could not fetch xcap document\n");
> goto error;
>
>
> Please find the attached xcap_server.c.new
>
> Best Regards,
> Laura
>
>
> On Thu, Sep 8, 2011 at 4:08 PM, Daniel-Constantin Mierla
> <miconda at gmail.com> wrote:
>> Hello Laura,
>>
>> thanks for testing and further patching. I just committed on master branch.
>> Are you working with 3.1? Just to see how fast to plan the backports.
>>
>> Cheers,
>> Daniel
>>
>> On 9/8/11 3:22 PM, laura testi wrote:
>>
>> Hi,
>> we have tested the patch, it does not work. We have done some minor
>> changes. Now it works.
>>
>> Here are the diff:
>>
>> diff -u xcap_misc.c xcap_misc.c_patch
>> --- xcap_misc.c 2011-09-08 14:23:37.000000000 +0200
>> +++ xcap_misc.c_patch 2011-09-08 14:29:24.000000000 +0200
>> @@ -465,7 +465,7 @@
>> goto error;
>> }
>> nodes = xpathObj->nodesetval;
>> - if(nodes==NULL)
>> + if(nodes==NULL || nodes->nodeNr==0 || nodes->nodeTab == NULL)
>> {
>> /* no selection for xpath expression */
>> LM_DBG("no selection for xpath expression [%s]\n",
>> xpaths->s);
>> @@ -488,7 +488,7 @@
>> }
>> *p = '/';
>> nodes = xpathObj->nodesetval;
>> - if(nodes==NULL)
>> + if(nodes==NULL || nodes->nodeNr==0 || nodes->nodeTab ==
>> NULL)
>> {
>> LM_DBG("no selection for xpath parent
>> expression [%s]\n",
>> xpaths->s);
>>
>>
>>
>> Please see the patch file in the attachment.
>>
>> Thanks again
>>
>> Best Regards,
>> Laura
>>
>> On Wed, Sep 7, 2011 at 10:12 PM, Daniel-Constantin Mierla
>> <miconda at gmail.com> wrote:
>>
>> Hello,
>>
>> inserting a new node seems not to be possible through xpath operations only,
>> so I made a patch to be able to add new nodes (entries) -- see it attached
>> -- it is for devel version (master branch), hopefully it applies clean to
>> 3.1 if you are using that version.
>>
>> There was no option to test it at all, I just made sure it compiles. Let me
>> know the results, if it runs but fails, send me the output with debug=4. If
>> all is ok, I will commit to git repository.
>>
>> Cheers,
>> Daniel
>>
>> On 9/7/11 3:21 PM, laura testi wrote:
>>
>> Hi,
>> we have patch the xcap_misc.h with "#define XCAP_MAX_URI_SIZE 255"
>> which was 127, and extended the length of field doc_uri in the xcap
>> table to 256 from 128. Now the errror is gone.
>>
>> After that we have done some test with curl, following are the results:
>> - DELETE an Entry from the list: OK
>> i.e.: curl -X DELETE
>>
>> http://<ip>:5060/xcap-root/resource-lists/users/sip:user at domain/index/~~/resource-lists/list/entry%5b at uri=%22sip:u1 at d1%22%5d
>> after this command, the contact u1 at d1 is removed from the contact list.
>>
>> - UPDATE an Entry from the list: OK
>> i.e.,
>> curl -T entry.xml -X PUT
>>
>> http://<ip>:5060/xcap-root/resource-lists/users/sip:user at domain/index/~~/resource-lists/list%5b at name=%22RootGroup%22%5d/entry%5b at uri=%22sip:u2 at d2%22%5d
>>
>> if the sip:u2 at d2 entry exists, the entry is replaced with the entry in
>> the entry.xml (<entry
>> uri="sip:u3 at d3"><display-name>u3</display-name></entry>), which is
>> sip:u3 at d3
>>
>> if the sip:u2 at d2 entry is not exists, the entry of u3 at d3 is not added
>> to the list!!! follow the rfc4825, it should be added to the list if
>> the entry does not exits.
>>
>> - Add an Entry from the list: KO
>> i.e.,
>> curl -T entry.xml -X PUT
>>
>> http://<ip>:5060/xcap-root/resource-lists/users/sip:user at domain/index/~~/resource-lists/list%5b at name=%22RootGroup%22%5d/entry%5b at uri=%22sip:u2 at d2%22%5d
>>
>> the entry.xml is (<entry
>> uri="sip:u2 at d2"><display-name>u2</display-name></entry>)
>>
>> the entry sip:u2 at d2 was not in the list before the command and is not
>> added after the command.
>>
>> I think it's a bug of xcap server.
>>
>>
>> Many thanks!
>>
>> Laura
>>
>>
>>
>>
>> On Wed, Sep 7, 2011 at 12:17 PM, Juha Heinanen<jh at tutpro.com> wrote:
>>
>> Daniel-Constantin Mierla writes:
>>
>> Feel free to do it, however I think it has to be reviewed where is used,
>> though -- it should be safe even size in db is longer as long as the
>> insert is done by kamailio itself, not sure we have affected cases when
>> the insert is done by external apps, so better double check...
>>
>> every module that reads data from db should check size of received
>> string so that it is not longer than space reserved for it in data
>> structures. i hope that no module assumes that received data cannot be
>> bigger than what db schema says.
>>
>> -- juha
--
Daniel-Constantin Mierla -- http://www.asipto.com
Kamailio Advanced Training, Oct 10-13, Berlin: http://asipto.com/u/kat
http://linkedin.com/in/miconda -- http://twitter.com/miconda
More information about the sr-users
mailing list