subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From rhuij...@apache.org
Subject svn commit: r1499386 - in /subversion/trunk/subversion/libsvn_ra_serf: ra_serf.h util.c xml.c
Date Wed, 03 Jul 2013 13:13:49 GMT
Author: rhuijben
Date: Wed Jul  3 13:13:49 2013
New Revision: 1499386

URL: http://svn.apache.org/r1499386
Log:
Improve transition based xml parser validations a bit by checking for opening
and closing of a document element. Also improve an error message for this
parser, to help me in debugging.

* subversion/libsvn_ra_serf/ra_serf.h
  (svn_ra_serf__xml_context_destroy): Rename to ...
  (svn_ra_serf__xml_context_done): ... this.

* subversion/libsvn_ra_serf/util.c
  (expat_response_handler): Avoid duplicated code by using an at_eof flag.
     Call svn_ra_serf__xml_context_done() to verify clean parser shutdown.

* subversion/libsvn_ra_serf/xml.c
  (svn_ra_serf__xml_context_destroy): Rename to...
  (svn_ra_serf__xml_context_done): ... this and add verification on the
    final state of the transition parser.

  (svn_ra_serf__xml_cb_start): Improve error message.

Modified:
    subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
    subversion/trunk/subversion/libsvn_ra_serf/util.c
    subversion/trunk/subversion/libsvn_ra_serf/xml.c

Modified: subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h?rev=1499386&r1=1499385&r2=1499386&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h Wed Jul  3 13:13:49 2013
@@ -801,10 +801,10 @@ svn_ra_serf__xml_context_create(
   void *baton,
   apr_pool_t *result_pool);
 
-/* Destroy all subpools for this structure. */
-void
-svn_ra_serf__xml_context_destroy(
-  svn_ra_serf__xml_context_t *xmlctx);
+/* Verifies if the parsing completed successfully and destroys
+   all subpools. */
+svn_error_t *
+svn_ra_serf__xml_context_done(svn_ra_serf__xml_context_t *xmlctx);
 
 /* Construct a handler with the response function/baton set up to parse
    a response body using the given XML context. The handler and its

Modified: subversion/trunk/subversion/libsvn_ra_serf/util.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/util.c?rev=1499386&r1=1499385&r2=1499386&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/util.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/util.c Wed Jul  3 13:13:49 2013
@@ -2540,10 +2540,13 @@ expat_response_handler(serf_request_t *r
       const char *data;
       apr_size_t len;
       int expat_status;
+      svn_boolean_t at_eof = FALSE;
 
       status = serf_bucket_read(response, PARSE_CHUNK_SIZE, &data, &len);
       if (SERF_BUCKET_READ_ERROR(status))
         return svn_ra_serf__wrap_err(status, NULL);
+      else if (APR_STATUS_IS_EOF(status))
+        at_eof = TRUE;
 
 #if 0
       /* ### move restart/skip into the core handler  */
@@ -2555,7 +2558,15 @@ expat_response_handler(serf_request_t *r
 
       /* ### should we have an IGNORE_ERRORS flag like the v1 parser?  */
 
-      expat_status = XML_Parse(ectx->parser, data, (int)len, 0 /* isFinal */);
+      expat_status = XML_Parse(ectx->parser, data, (int)len,
+                               at_eof /* isFinal */);
+
+      if (at_eof)
+        {
+          /* Release xml parser state/tables. */
+          apr_pool_cleanup_run(ectx->cleanup_pool, &ectx->parser,
+                               xml_parser_cleanup);
+        }
 
       /* We need to check INNER_ERROR first. This is an error from the
          callbacks that has been "dropped off" for us to retrieve. On
@@ -2564,12 +2575,7 @@ expat_response_handler(serf_request_t *r
 
          If an error is not present, THEN we go ahead and look for parsing
          errors.  */
-      if (ectx->inner_error)
-        {
-          apr_pool_cleanup_run(ectx->cleanup_pool, &ectx->parser,
-                               xml_parser_cleanup);
-          return svn_error_trace(ectx->inner_error);
-        }
+      SVN_ERR(ectx->inner_error);
       if (expat_status == XML_STATUS_ERROR)
         return svn_error_createf(SVN_ERR_XML_MALFORMED,
                                  ectx->inner_error,
@@ -2581,18 +2587,10 @@ expat_response_handler(serf_request_t *r
 
       /* The parsing went fine. What has the bucket told us?  */
 
-      if (APR_STATUS_IS_EOF(status))
+      if (at_eof)
         {
-          /* Tell expat we've reached the end of the content. Ignore the
-             return status. We just don't care.  */
-          (void) XML_Parse(ectx->parser, NULL, 0, 1 /* isFinal */);
-
-          svn_ra_serf__xml_context_destroy(ectx->xmlctx);
-          apr_pool_cleanup_run(ectx->cleanup_pool, &ectx->parser,
-                               xml_parser_cleanup);
-
-          /* ### should check XMLCTX to see if it has returned to the
-             ### INITIAL state. we may have ended early...  */
+          /* Make sure we actually got xml and clean up after parsing */
+          SVN_ERR(svn_ra_serf__xml_context_done(ectx->xmlctx));
         }
 
       if (status && !SERF_BUCKET_READ_ERROR(status))

Modified: subversion/trunk/subversion/libsvn_ra_serf/xml.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/xml.c?rev=1499386&r1=1499385&r2=1499386&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/xml.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/xml.c Wed Jul  3 13:13:49 2013
@@ -458,11 +458,50 @@ lazy_create_pool(void *baton)
   return xes->state_pool;
 }
 
-void
-svn_ra_serf__xml_context_destroy(
-  svn_ra_serf__xml_context_t *xmlctx)
+svn_error_t *
+svn_ra_serf__xml_context_done(svn_ra_serf__xml_context_t *xmlctx)
 {
+  if (xmlctx->current->prev)
+    {
+      /* Probably unreachable as this would be an xml parser error */
+      return svn_error_createf(SVN_ERR_XML_MALFORMED, NULL,
+                               _("XML stream truncated: closing '%s' missing"),
+                               xmlctx->current->tag.name);
+    }
+  else if (! xmlctx->free_states)
+    {
+      /* If we have no items on the free_states list, we didn't push anything,
+         which tells us that we found an empty xml body */
+      const svn_ra_serf__xml_transition_t *scan;
+      const svn_ra_serf__xml_transition_t *document = NULL;
+      const char *msg;
+
+      for (scan = xmlctx->ttable; scan->ns != NULL; ++scan)
+        {
+          if (scan->from_state == XML_STATE_INITIAL)
+            {
+              if (document != NULL)
+                {
+                  document = NULL; /* Multiple document elements defined */
+                  break;
+                }
+              document = scan;
+            }
+        }
+
+      if (document)
+        msg = apr_psprintf(xmlctx->scratch_pool, "'%s' element not found",
+                            document->name);
+      else
+        msg = _("document element not found");
+
+      return svn_error_createf(SVN_ERR_XML_MALFORMED, NULL,
+                               _("XML stream truncated: %s"),
+                               msg);
+    }
+
   svn_pool_destroy(xmlctx->scratch_pool);
+  return SVN_NO_ERROR;
 }
 
 svn_ra_serf__xml_context_t *
@@ -677,10 +716,11 @@ svn_ra_serf__xml_cb_start(svn_ra_serf__x
                   name = *saveattr;
                   value = svn_xml_get_attr_value(name, attrs);
                   if (value == NULL)
-                    return svn_error_createf(SVN_ERR_XML_ATTRIB_NOT_FOUND,
-                                             NULL,
-                                             _("Missing XML attribute: '%s'"),
-                                             name);
+                    return svn_error_createf(
+                                SVN_ERR_XML_ATTRIB_NOT_FOUND,
+                                NULL,
+                                _("Missing XML attribute '%s' on '%s' element"),
+                                name, scan->name);
                 }
 
               if (value)



Mime
View raw message