Apart from Kannan pinging his own update; This submission update has received no new comments.
Gavin "Beau" Baumanis On 07/01/2010, at 11:40 PM, Kannan wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > Julian Foad wrote: > [..] >> I looked for assignments to the "activity_coll" field, and found it in >> the function end_element(), where it copies it from the "cdata" field. I >> think that is where the canonicalization should be done. > > My bad, misinterpreted `end_element()' as neon's method! > >> And here: >> >>> * props.c >>> (svn_ra_neon__get_baseline_info, svn_ra_neon__get_one_prop): Same. >> >> The function ..._get_baseline_info() gets its bc_url from a hash created >> by the function svn_ra_neon__get_baseline_props(), so I think that's >> where the change needs to be. >> >> The function "get_one_prop" is not specifically for getting a baseline >> URL, it is a generic function for getting any property, so I think you >> should not be changing that function. > > Tracing these back further, the value gets set in `end_element()'. This > fixes one more instance of non-canonical URL in `get_version_url()', > thus fixing all instances of non-canonical URLs AFAICS. Thank you for > the feedback, Julian. Attaching the updated patch herewith. > > [[[ > Log: > Ensure the URLs are always canonical. > > [ in subversion/libsvn_ra_neon ] > > * util.c > (svn_ra_neon__request_get_location): Canonicalize the 'BASE URL' as > per the rule. > > * props.c > (end_element): Same. > > * options.c > (end_element): Same. > > Found by: stsp > Suggested by: stsp, julianfoad > Patch by: Kannan R <kann...@collab.net> > ]]] > > If the above patch seems fine, the patch to upgrade ..add_component() to > ..add_component2() can be found in [1]. > > [1]-http://mail-archives.apache.org/mod_mbox/subversion-dev/200912.mbox/%3c4b2b12ea.5000...@collab.net%3e > > - -- > Thanks & Regards, > Kannan > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v1.4.6 (GNU/Linux) > Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ > > iQEVAwUBS0XWUHlTqcY7ytmIAQIRnwf+LBmMhAKRPwCbWO2tmtzkhuQ9Bx7P1wlK > lGFhyhUf82InW6uEYqMP9UaihxPBeSDr5Cqec92HN8xV/O1j0366pBdSJ/1sXvrn > hg8U3toh9GSbIm5FqQ9WDWgOjYPG0VSr9pz/rFi+6fL9DkrSWqfcfHLZOJ4YIYQW > jPpLhRFVuypQDxpeoOpSuh3PHPg+zovaEsGCNepfqpwiVITvK3B74r8zq/sYd6SL > MF56QKDjgfFJAbD8W9SlA+y6aYJQm1ZxUMV9B2FdBSx8hv+i7SM3U6UTinQizUs8 > 8zutd2q7i7TA1jyqyZGRKx+4pProUbeIO1mUsRJrpwjaYlzssjRzRA== > =8XvR > -----END PGP SIGNATURE----- > Index: subversion/libsvn_ra_neon/util.c > =================================================================== > --- subversion/libsvn_ra_neon/util.c (revision 896759) > +++ subversion/libsvn_ra_neon/util.c (working copy) > @@ -1490,5 +1490,5 @@ > apr_pool_t *pool) > { > const char *val = ne_get_response_header(request->ne_req, "Location"); > - return val ? apr_pstrdup(pool, val) : NULL; > + return val ? svn_uri_canonicalize(val, pool) : NULL; > } > Index: subversion/libsvn_ra_neon/options.c > =================================================================== > --- subversion/libsvn_ra_neon/options.c (revision 896759) > +++ subversion/libsvn_ra_neon/options.c (working copy) > @@ -107,7 +107,9 @@ > options_ctx_t *oc = baton; > > if (state == ELEM_href) > - oc->activity_coll = svn_string_create_from_buf(oc->cdata, oc->pool); > + oc->activity_coll = > svn_string_create(svn_uri_canonicalize(oc->cdata->data, > + oc->pool), > + oc->pool); > > return SVN_NO_ERROR; > } > Index: subversion/libsvn_ra_neon/props.c > =================================================================== > --- subversion/libsvn_ra_neon/props.c (revision 896759) > +++ subversion/libsvn_ra_neon/props.c (working copy) > @@ -359,7 +359,7 @@ > const elem_defn *parent_defn; > const elem_defn *defn; > ne_status status; > - const char *cdata = pc->cdata->data; > + const char *cdata = svn_uri_canonicalize(pc->cdata->data, pc->pool); > > switch (state) > {
