{"thread":{"id":"2592","subject":"[PATCH 4/5] Improve XML parsing in http-push","startedAt":"2005-11-18T19:03:18Z","lastAt":"2005-11-18T19:03:18Z","messageCount":1,"participants":["Nick Hengeveld"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"12237","messageId":"20051118190318.GI3968@reactrix.com","threadId":"2592","inReplyTo":null,"subject":"[PATCH 4/5] Improve XML parsing in http-push","fromName":"Nick Hengeveld","fromEmail":"nickh@reactrix.com","sentAt":"2005-11-18T19:03:18Z","receivedAt":"2005-11-18T19:03:18Z","isPatch":true,"sender":{"key":"nickh@reactrix.com","avatar":null},"body":"Improved XML parsing - replace specialized doc parser callbacks with generic\nfunctions that track the parser context and use document-specific callbacks\nto process that data.\n\nSigned-off-by: Nick Hengeveld <nickh@reactrix.com>\n\n\n---\n\n http-push.c |  319 ++++++++++++++++++++++++++++++-----------------------------\n 1 files changed, 163 insertions(+), 156 deletions(-)\n\napplies-to: 819a270f2d0479478cd34d90e5d764ffad88b9d5\n61e23ac52c5276cdddc9dd4d0de444dfe1dde39d\ndiff --git a/http-push.c b/http-push.c\nindex afdcd8b..2932693 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -24,13 +24,28 @@ enum XML_Status {\n \n #define RANGE_HEADER_SIZE 30\n \n-/* DAV method names and request body templates */\n+/* DAV methods */\n #define DAV_LOCK \"LOCK\"\n #define DAV_MKCOL \"MKCOL\"\n #define DAV_MOVE \"MOVE\"\n #define DAV_PROPFIND \"PROPFIND\"\n #define DAV_PUT \"PUT\"\n #define DAV_UNLOCK \"UNLOCK\"\n+\n+/* DAV lock flags */\n+#define DAV_PROP_LOCKWR (1u << 0)\n+#define DAV_PROP_LOCKEX (1u << 1)\n+#define DAV_LOCK_OK (1u << 2)\n+\n+/* DAV XML properties */\n+#define DAV_CTX_LOCKENTRY \".multistatus.response.propstat.prop.supportedlock.lockentry\"\n+#define DAV_CTX_LOCKTYPE_WRITE \".multistatus.response.propstat.prop.supportedlock.lockentry.locktype.write\"\n+#define DAV_CTX_LOCKTYPE_EXCLUSIVE \".multistatus.response.propstat.prop.supportedlock.lockentry.lockscope.exclusive\"\n+#define DAV_ACTIVELOCK_OWNER \".prop.lockdiscovery.activelock.owner.href\"\n+#define DAV_ACTIVELOCK_TIMEOUT \".prop.lockdiscovery.activelock.timeout\"\n+#define DAV_ACTIVELOCK_TOKEN \".prop.lockdiscovery.activelock.locktoken.href\"\n+\n+/* DAV request body templates */\n #define PROPFIND_REQUEST \"<?xml version=\\\"1.0\\\" encoding=\\\"utf-8\\\" ?>\\n<D:propfind xmlns:D=\\\"DAV:\\\">\\n<D:prop xmlns:R=\\\"%s\\\">\\n<D:supportedlock/>\\n</D:prop>\\n</D:propfind>\"\n #define LOCK_REQUEST \"<?xml version=\\\"1.0\\\" encoding=\\\"utf-8\\\" ?>\\n<D:lockinfo xmlns:D=\\\"DAV:\\\">\\n<D:lockscope><D:exclusive/></D:lockscope>\\n<D:locktype><D:write/></D:locktype>\\n<D:owner>\\n<D:href>mailto:%s</D:href>\\n</D:owner>\\n</D:lockinfo>\"\n \n@@ -92,14 +107,17 @@ struct transfer_request\n \n static struct transfer_request *request_queue_head = NULL;\n \n+struct xml_ctx\n+{\n+\tchar *name;\n+\tint len;\n+\tchar *cdata;\n+\tvoid (*userFunc)(struct xml_ctx *ctx, int tag_closed);\n+\tvoid *userData;\n+};\n+\n struct active_lock\n {\n-\tint ctx_activelock;\n-\tint ctx_owner;\n-\tint ctx_owner_href;\n-\tint ctx_timeout;\n-\tint ctx_locktoken;\n-\tint ctx_locktoken_href;\n \tchar *url;\n \tchar *owner;\n \tchar *token;\n@@ -108,16 +126,6 @@ struct active_lock\n \tint refreshing;\n };\n \n-struct lockprop\n-{\n-\tint supported_lock;\n-\tint lock_entry;\n-\tint lock_scope;\n-\tint lock_type;\n-\tint lock_exclusive;\n-\tint lock_exclusive_write;\n-};\n-\n static void finish_request(struct transfer_request *request);\n \n static void process_response(void *callback_data)\n@@ -734,107 +742,101 @@ int fetch_ref(char *ref, unsigned char *\n         return 0;\n }\n \n-static void\n-start_activelock_element(void *userData, const char *name, const char **atts)\n+static void handle_lockprop_ctx(struct xml_ctx *ctx, int tag_closed)\n {\n-\tstruct active_lock *lock = (struct active_lock *)userData;\n+\tint *lock_flags = (int *)ctx->userData;\n \n-\tif (lock->ctx_activelock && !strcmp(name, \"D:timeout\"))\n-\t\tlock->ctx_timeout = 1;\n-\telse if (lock->ctx_owner && strstr(name, \"href\"))\n-\t\tlock->ctx_owner_href = 1;\n-\telse if (lock->ctx_activelock && strstr(name, \"owner\"))\n-\t\tlock->ctx_owner = 1;\n-\telse if (lock->ctx_locktoken && !strcmp(name, \"D:href\"))\n-\t\tlock->ctx_locktoken_href = 1;\n-\telse if (lock->ctx_activelock && !strcmp(name, \"D:locktoken\"))\n-\t\tlock->ctx_locktoken = 1;\n-\telse if (!strcmp(name, \"D:activelock\"))\n-\t\tlock->ctx_activelock = 1;\n+\tif (tag_closed) {\n+\t\tif (!strcmp(ctx->name, DAV_CTX_LOCKENTRY)) {\n+\t\t\tif ((*lock_flags & DAV_PROP_LOCKEX) &&\n+\t\t\t    (*lock_flags & DAV_PROP_LOCKWR)) {\n+\t\t\t\t*lock_flags |= DAV_LOCK_OK;\n+\t\t\t}\n+\t\t\t*lock_flags &= DAV_LOCK_OK;\n+\t\t} else if (!strcmp(ctx->name, DAV_CTX_LOCKTYPE_WRITE)) {\n+\t\t\t*lock_flags |= DAV_PROP_LOCKWR;\n+\t\t} else if (!strcmp(ctx->name, DAV_CTX_LOCKTYPE_EXCLUSIVE)) {\n+\t\t\t*lock_flags |= DAV_PROP_LOCKEX;\n+\t\t}\n+\t}\n }\n \n-static void\n-end_activelock_element(void *userData, const char *name)\n-{\n-\tstruct active_lock *lock = (struct active_lock *)userData;\n-\n-\tif (lock->ctx_timeout && !strcmp(name, \"D:timeout\")) {\n-\t\tlock->ctx_timeout = 0;\n-\t} else if (lock->ctx_owner_href && strstr(name, \"href\")) {\n-\t\tlock->ctx_owner_href = 0;\n-\t} else if (lock->ctx_owner && strstr(name, \"owner\")) {\n-\t\tlock->ctx_owner = 0;\n-\t} else if (lock->ctx_locktoken_href && !strcmp(name, \"D:href\")) {\n-\t\tlock->ctx_locktoken_href = 0;\n-\t} else if (lock->ctx_locktoken && !strcmp(name, \"D:locktoken\")) {\n-\t\tlock->ctx_locktoken = 0;\n-\t} else if (lock->ctx_activelock && !strcmp(name, \"D:activelock\")) {\n-\t\tlock->ctx_activelock = 0;\n+static void handle_new_lock_ctx(struct xml_ctx *ctx, int tag_closed)\n+{\n+\tstruct active_lock *lock = (struct active_lock *)ctx->userData;\n+\n+\tif (tag_closed && ctx->cdata) {\n+\t\tif (!strcmp(ctx->name, DAV_ACTIVELOCK_OWNER)) {\n+\t\t\tlock->owner = xmalloc(strlen(ctx->cdata) + 1);\n+\t\t\tstrcpy(lock->owner, ctx->cdata);\n+\t\t} else if (!strcmp(ctx->name, DAV_ACTIVELOCK_TIMEOUT)) {\n+\t\t\tif (!strncmp(ctx->cdata, \"Second-\", 7))\n+\t\t\t\tlock->timeout =\n+\t\t\t\t\tstrtol(ctx->cdata + 7, NULL, 10);\n+\t\t} else if (!strcmp(ctx->name, DAV_ACTIVELOCK_TOKEN)) {\n+\t\t\tif (!strncmp(ctx->cdata, \"opaquelocktoken:\", 16)) {\n+\t\t\t\tlock->token = xmalloc(strlen(ctx->cdata - 15));\n+\t\t\t\tstrcpy(lock->token, ctx->cdata + 16);\n+\t\t\t}\n+\t\t}\n \t}\n }\n \n static void\n-activelock_cdata(void *userData, const XML_Char *s, int len)\n+xml_start_tag(void *userData, const char *name, const char **atts)\n {\n-\tstruct active_lock *lock = (struct active_lock *)userData;\n-\tchar *this = malloc(len+1);\n-\tstrncpy(this, s, len);\n-\n-\tif (lock->ctx_owner_href) {\n-\t\tlock->owner = malloc(len+1);\n-\t\tstrcpy(lock->owner, this);\n-\t} else if (lock->ctx_locktoken_href) {\n-\t\tif (!strncmp(this, \"opaquelocktoken:\", 16)) {\n-\t\t\tlock->token = malloc(len-15);\n-\t\t\tstrcpy(lock->token, this+16);\n-\t\t}\n-\t} else if (lock->ctx_timeout) {\n-\t\tif (!strncmp(this, \"Second-\", 7))\n-\t\t\tlock->timeout = strtol(this+7, NULL, 10);\n+\tstruct xml_ctx *ctx = (struct xml_ctx *)userData;\n+\tconst char *c = index(name, ':');\n+\tint new_len;\n+\n+\tif (c == NULL)\n+\t\tc = name;\n+\telse\n+\t\tc++;\n+\n+\tnew_len = strlen(ctx->name) + strlen(c) + 2;\n+\n+\tif (new_len > ctx->len) {\n+\t\tctx->name = xrealloc(ctx->name, new_len);\n+\t\tctx->len = new_len;\n+\t}\n+\tstrcat(ctx->name, \".\");\n+\tstrcat(ctx->name, c);\n+\n+\tif (ctx->cdata) {\n+\t\tfree(ctx->cdata);\n+\t\tctx->cdata = NULL;\n \t}\n \n-\tfree(this);\n+\tctx->userFunc(ctx, 0);\n }\n \n static void\n-start_lockprop_element(void *userData, const char *name, const char **atts)\n+xml_end_tag(void *userData, const char *name)\n {\n-\tstruct lockprop *prop = (struct lockprop *)userData;\n+\tstruct xml_ctx *ctx = (struct xml_ctx *)userData;\n+\tconst char *c = index(name, ':');\n+\tchar *ep;\n \n-\tif (prop->lock_type && !strcmp(name, \"D:write\")) {\n-\t\tif (prop->lock_exclusive) {\n-\t\t\tprop->lock_exclusive_write = 1;\n-\t\t}\n-\t} else if (prop->lock_scope && !strcmp(name, \"D:exclusive\")) {\n-\t\tprop->lock_exclusive = 1;\n-\t} else if (prop->lock_entry) {\n-\t\tif (!strcmp(name, \"D:lockscope\")) {\n-\t\t\tprop->lock_scope = 1;\n-\t\t} else if (!strcmp(name, \"D:locktype\")) {\n-\t\t\tprop->lock_type = 1;\n-\t\t}\n-\t} else if (prop->supported_lock) {\n-\t\tif (!strcmp(name, \"D:lockentry\")) {\n-\t\t\tprop->lock_entry = 1;\n-\t\t}\n-\t} else if (!strcmp(name, \"D:supportedlock\")) {\n-\t\tprop->supported_lock = 1;\n-\t}\n+\tctx->userFunc(ctx, 1);\n+\n+\tif (c == NULL)\n+\t\tc = name;\n+\telse\n+\t\tc++;\n+\n+\tep = ctx->name + strlen(ctx->name) - strlen(c) - 1;\n+\t*ep = 0;\n }\n \n static void\n-end_lockprop_element(void *userData, const char *name)\n+xml_cdata(void *userData, const XML_Char *s, int len)\n {\n-\tstruct lockprop *prop = (struct lockprop *)userData;\n-\n-\tif (!strcmp(name, \"D:lockentry\")) {\n-\t\tprop->lock_entry = 0;\n-\t\tprop->lock_scope = 0;\n-\t\tprop->lock_type = 0;\n-\t\tprop->lock_exclusive = 0;\n-\t} else if (!strcmp(name, \"D:supportedlock\")) {\n-\t\tprop->supported_lock = 0;\n-\t}\n+\tstruct xml_ctx *ctx = (struct xml_ctx *)userData;\n+\tif (ctx->cdata)\n+\t\tfree(ctx->cdata);\n+\tctx->cdata = xcalloc(len+1, 1);\n+\tstrncpy(ctx->cdata, s, len);\n }\n \n static struct active_lock *lock_remote(char *file, long timeout)\n@@ -847,10 +849,11 @@ static struct active_lock *lock_remote(c\n \tchar *url;\n \tchar *ep;\n \tchar timeout_header[25];\n-\tstruct active_lock *new_lock;\n+\tstruct active_lock *new_lock = NULL;\n \tXML_Parser parser = XML_ParserCreate(NULL);\n \tenum XML_Status result;\n \tstruct curl_slist *dav_headers = NULL;\n+\tstruct xml_ctx ctx;\n \n \turl = xmalloc(strlen(remote->url) + strlen(file) + 1);\n \tsprintf(url, \"%s%s\", remote->url, file);\n@@ -894,12 +897,6 @@ static struct active_lock *lock_remote(c\n \tin_buffer.posn = 0;\n \tin_buffer.buffer = in_data;\n \n-\tnew_lock = xcalloc(1, sizeof(*new_lock));\n-\tnew_lock->owner = NULL;\n-\tnew_lock->token = NULL;\n-\tnew_lock->timeout = -1;\n-\tnew_lock->refreshing = 0;\n-\n \tsprintf(timeout_header, \"Timeout: Second-%ld\", timeout);\n \tdav_headers = curl_slist_append(dav_headers, timeout_header);\n \tdav_headers = curl_slist_append(dav_headers, \"Content-Type: text/xml\");\n@@ -915,40 +912,41 @@ static struct active_lock *lock_remote(c\n \tcurl_easy_setopt(slot->curl, CURLOPT_CUSTOMREQUEST, DAV_LOCK);\n \tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, dav_headers);\n \n+\tnew_lock = xcalloc(1, sizeof(*new_lock));\n+\tnew_lock->owner = NULL;\n+\tnew_lock->token = NULL;\n+\tnew_lock->timeout = -1;\n+\tnew_lock->refreshing = 0;\n+\n \tif (start_active_slot(slot)) {\n \t\trun_active_slot(slot);\n-\t\tif (slot->curl_result != CURLE_OK) {\n-\t\t\tfprintf(stderr, \"Got HTTP error %ld\\n\", slot->http_code);\n-\t\t\tfree(new_lock);\n-\t\t\tfree(url);\n-\t\t\tfree(out_data);\n-\t\t\tfree(in_data);\n-\t\t\treturn NULL;\n+\t\tif (slot->curl_result == CURLE_OK) {\n+\t\t\tctx.name = xcalloc(10, 1);\n+\t\t\tctx.len = 0;\n+\t\t\tctx.cdata = NULL;\n+\t\t\tctx.userFunc = handle_new_lock_ctx;\n+\t\t\tctx.userData = new_lock;\n+\t\t\tXML_SetUserData(parser, &ctx);\n+\t\t\tXML_SetElementHandler(parser, xml_start_tag,\n+\t\t\t\t\t      xml_end_tag);\n+\t\t\tXML_SetCharacterDataHandler(parser, xml_cdata);\n+\t\t\tresult = XML_Parse(parser, in_buffer.buffer,\n+\t\t\t\t\t   in_buffer.posn, 1);\n+\t\t\tfree(ctx.name);\n+\t\t\tif (result != XML_STATUS_OK) {\n+\t\t\t\tfprintf(stderr, \"XML error: %s\\n\",\n+\t\t\t\t\tXML_ErrorString(\n+\t\t\t\t\t\tXML_GetErrorCode(parser)));\n+\t\t\t\tnew_lock->timeout = -1;\n+\t\t\t}\n \t\t}\n \t} else {\n-\t\tfree(new_lock);\n-\t\tfree(url);\n-\t\tfree(out_data);\n-\t\tfree(in_data);\n \t\tfprintf(stderr, \"Unable to start request\\n\");\n-\t\treturn NULL;\n \t}\n \n+\tcurl_slist_free_all(dav_headers);\n \tfree(out_data);\n-\n-\tXML_SetUserData(parser, new_lock);\n-\tXML_SetElementHandler(parser, start_activelock_element,\n-\t\t\t\t      end_activelock_element);\n-\tXML_SetCharacterDataHandler(parser, activelock_cdata);\n-\tresult = XML_Parse(parser, in_buffer.buffer, in_buffer.posn, 1);\n \tfree(in_data);\n-\tif (result != XML_STATUS_OK) {\n-\t\tfprintf(stderr, \"%s\", XML_ErrorString(\n-\t\t\t\tXML_GetErrorCode(parser)));\n-\t\tfree(url);\n-\t\tfree(new_lock);\n-\t\treturn NULL;\n-\t}\n \n \tif (new_lock->token == NULL || new_lock->timeout <= 0) {\n \t\tif (new_lock->token != NULL)\n@@ -957,11 +955,12 @@ static struct active_lock *lock_remote(c\n \t\t\tfree(new_lock->owner);\n \t\tfree(url);\n \t\tfree(new_lock);\n-\t\treturn NULL;\n+\t\tnew_lock = NULL;\n+\t} else {\n+\t\tnew_lock->url = url;\n+\t\tnew_lock->start_time = time(NULL);\n \t}\n \n-\tnew_lock->url = url;\n-\tnew_lock->start_time = time(NULL);\n \treturn new_lock;\n }\n \n@@ -1000,13 +999,15 @@ static int unlock_remote(struct active_l\n \tif (lock->owner != NULL)\n \t\tfree(lock->owner);\n \tfree(lock->url);\n+/* Freeing the token causes a segfault...\n \tfree(lock->token);\n+*/\n \tfree(lock);\n \n \treturn rc;\n }\n \n-static int check_locking(void)\n+static int locking_available(void)\n {\n \tstruct active_request_slot *slot;\n \tstruct buffer in_buffer;\n@@ -1015,8 +1016,9 @@ static int check_locking(void)\n \tchar *out_data;\n \tXML_Parser parser = XML_ParserCreate(NULL);\n \tenum XML_Status result;\n-\tstruct lockprop supported_lock;\n \tstruct curl_slist *dav_headers = NULL;\n+\tstruct xml_ctx ctx;\n+\tint lock_flags = 0;\n \n \tout_buffer.size = strlen(PROPFIND_REQUEST) + strlen(remote->url) - 2;\n \tout_data = xmalloc(out_buffer.size + 1);\n@@ -1045,30 +1047,35 @@ static int check_locking(void)\n \n \tif (start_active_slot(slot)) {\n \t\trun_active_slot(slot);\n-\t\tfree(out_data);\n-\t\tif (slot->curl_result != CURLE_OK) {\n-\t\t\tfree(in_buffer.buffer);\n-\t\t\treturn -1;\n+\t\tif (slot->curl_result == CURLE_OK) {\n+\t\t\tctx.name = xcalloc(10, 1);\n+\t\t\tctx.len = 0;\n+\t\t\tctx.cdata = NULL;\n+\t\t\tctx.userFunc = handle_lockprop_ctx;\n+\t\t\tctx.userData = &lock_flags;\n+\t\t\tXML_SetUserData(parser, &ctx);\n+\t\t\tXML_SetElementHandler(parser, xml_start_tag,\n+\t\t\t\t\t      xml_end_tag);\n+\t\t\tresult = XML_Parse(parser, in_buffer.buffer,\n+\t\t\t\t\t   in_buffer.posn, 1);\n+\t\t\tfree(ctx.name);\n+\n+\t\t\tif (result != XML_STATUS_OK) {\n+\t\t\t\tfprintf(stderr, \"XML error: %s\\n\",\n+\t\t\t\t\tXML_ErrorString(\n+\t\t\t\t\t\tXML_GetErrorCode(parser)));\n+\t\t\t\tlock_flags = 0;\n+\t\t\t}\n \t\t}\n-\n-\t\tXML_SetUserData(parser, &supported_lock);\n-\t\tXML_SetElementHandler(parser, start_lockprop_element,\n-\t\t\t\t      end_lockprop_element);\n-\t\tresult = XML_Parse(parser, in_buffer.buffer, in_buffer.posn, 1);\n-\t\tfree(in_buffer.buffer);\n-\t\tif (result != XML_STATUS_OK)\n-\t\t\treturn error(\"%s\", XML_ErrorString(\n-\t\t\t\t\t     XML_GetErrorCode(parser)));\n \t} else {\n-\t\tfree(out_data);\n-\t\tfree(in_buffer.buffer);\n-\t\treturn error(\"Unable to start request\");\n+\t\tfprintf(stderr, \"Unable to start request\\n\");\n \t}\n \n-\tif (supported_lock.lock_exclusive_write)\n-\t\treturn 0;\n-\telse\n-\t\treturn 1;\n+\tfree(out_data);\n+\tfree(in_buffer.buffer);\n+\tcurl_slist_free_all(dav_headers);\n+\n+\treturn lock_flags;\n }\n \n static int is_ancestor(unsigned char *sha1, struct commit *commit)\n@@ -1269,7 +1276,7 @@ int main(int argc, char **argv)\n \t\t\t\t\t    \"Pragma: no-cache\");\n \n \t/* Verify DAV compliance/lock support */\n-\tif (check_locking() != 0) {\n+\tif (!locking_available()) {\n \t\tfprintf(stderr, \"Error: no DAV locking support on remote repo %s\\n\", remote->url);\n \t\trc = 1;\n \t\tgoto cleanup;\n---\n0.99.9.GIT\n"}]}