You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by cm...@apache.org on 2012/06/21 21:40:15 UTC

svn commit: r1352659 - in /subversion/trunk/subversion/libsvn_ra_serf: property.c update.c

Author: cmpilato
Date: Thu Jun 21 19:40:14 2012
New Revision: 1352659

URL: http://svn.apache.org/viewvc?rev=1352659&view=rev
Log:
Give ra_serf the power to properly recognize non-existent properties
explicitly requested, take 2.  (That is, clawing my way desperately
out of this particular corner of obsession with What Is Right.)

* subversion/libsvn_ra_serf/property.c
  (prop_state_e): Add STATUS state.
  (propfind_context_t): Add 'ps_props' member.
  (propfind_ttable): Add record for STATUS state, and flip the bit
    that lets us handle the "propstat" closure with our custom
    callback.
  (parse_status_code): New helper function.
  (copy_into_ret_props): New callback function.
  (propfind_opened): When entering PROPSTAT state, create the
    'ps_props' hash.
  (propfind_closed): Handle the STATUS closure, parsing the status
    code and leaving a note if it indicates that the property value
    isn't worthy of remembrance.  In the PROPVAL closure, store
    properties temporarily in the new 'ps_props' hash.  Now handle the
    PROPSTAT closure, where we can finally consult that status note
    and copy keepable properties from the 'ps_props' hash to the
    'ret_props' hash (and destroy the 'ps_props' hash).

* subversion/libsvn_ra_serf/update.c
  (try_get_wc_contents): Let empty-string checksums return to raising
    red flags.

Modified:
    subversion/trunk/subversion/libsvn_ra_serf/property.c
    subversion/trunk/subversion/libsvn_ra_serf/update.c

Modified: subversion/trunk/subversion/libsvn_ra_serf/property.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/property.c?rev=1352659&r1=1352658&r2=1352659&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/property.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/property.c Thu Jun 21 19:40:14 2012
@@ -46,6 +46,7 @@ typedef enum prop_state_e {
   RESPONSE,
   HREF,
   PROPSTAT,
+  STATUS,
   PROP,
   PROPVAL,
   COLLECTION,
@@ -86,6 +87,13 @@ typedef struct propfind_context_t {
    */
   apr_hash_t *ret_props;
 
+  /* hash table containing all the properties associated with the
+   * "current" <propstat> tag.  These will get copied into RET_PROPS
+   * if the status code similarly associated indicates that they are
+   * "good"; otherwise, they'll get discarded.
+   */
+  apr_hash_t *ps_props;
+
   /* If not-NULL, add us to this list when we're done. */
   svn_ra_serf__list_t **done_list;
 
@@ -107,7 +115,10 @@ static const svn_ra_serf__xml_transition
     TRUE, { NULL }, TRUE },
 
   { RESPONSE, D_, "propstat", PROPSTAT,
-    FALSE, { NULL }, FALSE },
+    FALSE, { NULL }, TRUE },
+
+  { PROPSTAT, D_, "status", STATUS,
+    TRUE, { NULL }, TRUE },
 
   { PROPSTAT, D_, "prop", PROP,
     FALSE, { NULL }, FALSE },
@@ -125,6 +136,46 @@ static const svn_ra_serf__xml_transition
 };
 
 
+/* Return the HTTP status code contained in STATUS_LINE, or 0 if
+   there's a problem parsing it. */
+static int parse_status_code(const char *status_line)
+{
+  /* STATUS_LINE should be of form: "HTTP/1.1 200 OK" */
+  if (status_line[0] == 'H' &&
+      status_line[1] == 'T' &&
+      status_line[2] == 'T' &&
+      status_line[3] == 'P' &&
+      status_line[4] == '/' &&
+      (status_line[5] >= '0' && status_line[5] <= '9') &&
+      status_line[6] == '.' &&
+      (status_line[7] >= '0' && status_line[7] <= '9') &&
+      status_line[8] == ' ')
+    {
+      char *reason;
+
+      return apr_strtoi64(status_line + 8, &reason, 10);
+    }
+  return 0;
+}
+
+
+/* Conforms to svn_ra_serf__path_rev_walker_t  */
+static svn_error_t *
+copy_into_ret_props(void *baton,
+                    const char *path, apr_ssize_t path_len,
+                    const char *ns, apr_ssize_t ns_len,
+                    const char *name, apr_ssize_t name_len,
+                    const svn_string_t *val,
+                    apr_pool_t *pool)
+{
+  propfind_context_t *ctx = baton;
+
+  svn_ra_serf__set_ver_prop(ctx->ret_props, path, ctx->rev, ns, name,
+                            val, ctx->pool);
+  return SVN_NO_ERROR;
+}
+
+
 /* Conforms to svn_ra_serf__xml_opened_t  */
 static svn_error_t *
 propfind_opened(svn_ra_serf__xml_estate_t *xes,
@@ -133,11 +184,17 @@ propfind_opened(svn_ra_serf__xml_estate_
                 const svn_ra_serf__dav_props_t *tag,
                 apr_pool_t *scratch_pool)
 {
+  propfind_context_t *ctx = baton;
+
   if (entered_state == PROPVAL)
     {
       svn_ra_serf__xml_note(xes, PROPVAL, "ns", tag->namespace);
       svn_ra_serf__xml_note(xes, PROPVAL, "name", tag->name);
     }
+  else if (entered_state == PROPSTAT)
+    {
+      ctx->ps_props = apr_hash_make(ctx->pool);
+    }
 
   return SVN_NO_ERROR;
 }
@@ -193,7 +250,17 @@ propfind_closed(svn_ra_serf__xml_estate_
     {
       svn_ra_serf__xml_note(xes, PROPVAL, "altvalue", cdata->data);
     }
-  else
+  else if (leaving_state == STATUS)
+    {
+      /* Parse the status field, and remember if this is a property
+         that we wish to ignore.  (Typically, if it's not a 200, the
+         status will be 404 to indicate that a property we
+         specifically requested from the server doesn't exist.)  */
+      int status = parse_status_code(cdata->data);
+      if (status != 200)
+        svn_ra_serf__xml_note(xes, PROPSTAT, "ignore-prop", "*");
+    }
+  else if (leaving_state == PROPVAL)
     {
       const char *encoding = apr_hash_get(attrs, "V:encoding",
                                           APR_HASH_KEY_STRING);
@@ -204,8 +271,6 @@ propfind_closed(svn_ra_serf__xml_estate_
       const char *name;
       const char *altvalue;
 
-      SVN_ERR_ASSERT(leaving_state == PROPVAL);
-
       if (encoding)
         {
           if (strcmp(encoding, "base64") != 0)
@@ -225,7 +290,16 @@ propfind_closed(svn_ra_serf__xml_estate_
 
       /* The current path sits on the RESPONSE state. Gather up all the
          state from this PROPVAL to the (grandparent) RESPONSE state,
-         and grab the path from there.  */
+         and grab the path from there.
+
+         Now, it would be nice if we could, at this point, know that
+         the status code for this property indicated a problem -- then
+         we could simply bail out here and ignore the property.
+         Sadly, though, we might get the status code *after* we get
+         the property value.  So we'll carry on with our processing
+         here, setting the property and value as expected.  Once we
+         know for sure the status code associate with the property,
+         we'll decide its fate.  */
       gathered = svn_ra_serf__xml_gather_since(xes, RESPONSE);
 
       /* These will be dup'd into CTX->POOL, as necessary.  */
@@ -241,10 +315,35 @@ propfind_closed(svn_ra_serf__xml_estate_
       if (altvalue != NULL)
         val_str = svn_string_create(altvalue, ctx->pool);
 
-      svn_ra_serf__set_ver_prop(ctx->ret_props,
+      svn_ra_serf__set_ver_prop(ctx->ps_props,
                                 path, ctx->rev, ns, name, val_str,
                                 ctx->pool);
     }
+  else
+    {
+      apr_hash_t *gathered;
+      const char *path;
+
+      SVN_ERR_ASSERT(leaving_state == PROPSTAT);
+
+      gathered = svn_ra_serf__xml_gather_since(xes, PROPSTAT);
+
+      path = apr_hash_get(gathered, "path", APR_HASH_KEY_STRING);
+      if (path == NULL)
+        path = ctx->path;
+
+      /* If we've squirreled away a note that says we want to ignore
+         these properties, we'll do so.  Otherwise, we need to copy
+         them from the temporary hash into the ctx->ret_props hash. */
+      if (! apr_hash_get(gathered, "ignore-prop", APR_HASH_KEY_STRING))
+        {
+          SVN_ERR(svn_ra_serf__walk_all_paths(ctx->ps_props, ctx->rev,
+                                              copy_into_ret_props, ctx,
+                                              scratch_pool));
+        }
+
+      ctx->ps_props = NULL;
+    }
 
   return SVN_NO_ERROR;
 }

Modified: subversion/trunk/subversion/libsvn_ra_serf/update.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/update.c?rev=1352659&r1=1352658&r2=1352659&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/update.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/update.c Thu Jun 21 19:40:14 2012
@@ -2862,30 +2862,14 @@ try_get_wc_contents(svn_boolean_t *found
   svn_props = apr_hash_get(props, SVN_DAV_PROP_NS_DAV, APR_HASH_KEY_STRING);
   if (!svn_props)
     {
-      /* No checksum property in response. */
+      /* No properties -- therefore no checksum property -- in response. */
       return SVN_NO_ERROR;
     }
 
-  /* If we are talking to an old server, then the sha1-checksum property
-     will not exist. In our property parsing code, we don't bother to
-     check the <status> element which would indicate a 404. That section
-     needs to name the 404'd property, so our parsing code only sees:
-
-       <g0:sha1-checksum/>
-
-     That is parsed as an empty string value for the property.
-
-     When checking the property, if it is missing (NULL), or the above
-     empty string, then we know the property doesn't exist.
-
-     Strictly speaking, we could start parsing <status> and omit any
-     properties that were 404'd. But the 404 only happens when we ask
-     for a specific property, and it is easier to just check for the
-     empty string. Since it isn't a legal value in this case, we won't
-     get confused with a truly existent property.  */
   sha1_checksum_prop = svn_prop_get_value(svn_props, "sha1-checksum");
-  if (sha1_checksum_prop == NULL || *sha1_checksum_prop == '\0')
+  if (sha1_checksum_prop == NULL)
     {
+      /* No checksum property in response. */
       return SVN_NO_ERROR;
     }