On 06/20/2012 02:32 PM, Greg Stein wrote: > On Wed, Jun 20, 2012 at 2:30 PM, C. Michael Pilato <[email protected]> > wrote: >> On 06/20/2012 02:24 PM, [email protected] wrote: >>> + /* 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. */ >> >> I remember writing ra_neon's PROPFIND parsing logic to parse the >> per-property status field. Surely it can't be *that hard* to do the right >> thing here, and stop conflating the empty string with a NULL one. > > 404 can only happen for properties we specifically ask for. They are > always there. > > Except for this one, on old servers. > > I'm not going to write a hundred lines of code when an empty string > test works just fine.
Can you at least then give a quick review on this approach for a more proper
fix? I've not tested it ... just trying to see if I can grok your new
magical XML handling stuff (which is pretty sweet, by the way!)
Index: subversion/libsvn_ra_serf/property.c
===================================================================
--- subversion/libsvn_ra_serf/property.c (revision 1352206)
+++ subversion/libsvn_ra_serf/property.c (working copy)
@@ -193,6 +193,10 @@
{
svn_ra_serf__xml_note(xes, PROPVAL, "altvalue", cdata->data);
}
+ else if (leaving_state == PROPSTAT)
+ {
+ svn_ra_serf__xml_note(xes, PROPVAL, "propstat", cdata->data);
+ }
else
{
const char *encoding = apr_hash_get(attrs, "V:encoding",
@@ -202,10 +206,22 @@
const char *path;
const char *ns;
const char *name;
+ const char *propstat;
const char *altvalue;
SVN_ERR_ASSERT(leaving_state == PROPVAL);
+ /* Check the status of the property. If it isn't 200, it isn't
+ interesting to us. */
+ propstat = apr_hash_get(attrs, "propstat", APR_HASH_KEY_STRING);
+ if (propstat)
+ {
+ apr_int64_t status;
+ SVN_ERR(svn_cstring_atoi64(&status, propstat));
+ if (status != 200)
+ return SVN_NO_ERROR;
+ }
+
if (encoding)
{
if (strcmp(encoding, "base64") != 0)
--
C. Michael Pilato <[email protected]>
CollabNet <> www.collab.net <> Enterprise Cloud Development
signature.asc
Description: OpenPGP digital signature
