Kannan wrote: > Assertion failure occurred due to a non-canonical base URL passed to > `svn_path_url_add_component2()'. Despite having canonicalized them > wherever they are generated, the reason for this was that in > `end_element()' of props.c, canonicalization was done where the url was > assigned: > > <snip> > > return assign_rsrc_url(pc->rsrc, svn_uri_canonicalize(cdata, pc->pool), > pc->pool);
Yes, because in this code path we know that the value of the variable 'cdata' is expected to be a URL. > </snip> > > But there's one more place(which I missed to notice) where the value of > `cdata'(non-canonical url) is used to assigned the URL, for those > files/dirs who've parent info(cp/mv operations). So the non-canonical > URL was this one and hence the failure. OK, that's good: you've found the other place where it needs to be canonicalized. > Attached herewith is the patch which canonicalizes `cdata' where its > initialized as proposed in [1]. But that's not right. Like I said before, the variable holds different kinds of data at different times. (If we this variable was intended to always hold a URL, we would want the variable's name to indicate so.) It is wrong to call svn_uri_canonicalize() on a string that is not known to be a URL. Depending on what the string is, that might change it in some undesired way. > Though there's one more place: > > <snip> > > case ELEM_status: > /* Parse the <status> tag's CDATA for a status code. */ > if (ne_parse_statusline(cdata, &status)) > return svn_error_create(SVN_ERR_XML_MALFORMED, NULL, NULL); > > </snip> > > that does not need the canonicalized value, thought its better to do the > canonicalization in just one place. No. (And there are several other places where the variable is used in that function, where the value it holds is something other than a URL.) - Julian
