You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by gs...@apache.org on 2012/06/20 13:08:46 UTC
svn commit: r1352044 - /subversion/trunk/subversion/libsvn_ra_serf/update.c
Author: gstein
Date: Wed Jun 20 11:08:45 2012
New Revision: 1352044
URL: http://svn.apache.org/viewvc?rev=1352044&view=rev
Log:
Followup to r1350584: a missing sha1-checksum property can also be the
empty string (e.g. <S:sha1-checksum/>)
* subversion/libsvn_ra_serf/update.c:
(try_get_wc_contents): look for both missing and empty string on the
sha1-checksum property
Modified:
subversion/trunk/subversion/libsvn_ra_serf/update.c
Modified: subversion/trunk/subversion/libsvn_ra_serf/update.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/update.c?rev=1352044&r1=1352043&r2=1352044&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/update.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/update.c Wed Jun 20 11:08:45 2012
@@ -2874,9 +2874,9 @@ try_get_wc_contents(svn_boolean_t *found
sha1_checksum_prop = svn_prop_get_value(svn_props, "sha1-checksum");
- if (!sha1_checksum_prop)
+ if (sha1_checksum_prop == NULL || *sha1_checksum_prop == '\0')
{
- /* No checksum property in response. */
+ /* No checksum property in response (missing or empty string). */
return SVN_NO_ERROR;
}
Re: svn commit: r1352044 - /subversion/trunk/subversion/libsvn_ra_serf/update.c
Posted by Greg Stein <gs...@gmail.com>.
On Wed, Jun 20, 2012 at 8:08 AM, Ivan Zhakov <iv...@visualsvn.com> wrote:
> On Wed, Jun 20, 2012 at 3:12 PM, Greg Stein <gs...@gmail.com> wrote:
>> Stefan: this should fix your "cat" problem. If the server is old, then
>> it reports the property as missing. We don't examine the specific
>> status results (tho we should, but then again: we don't talk to any
>> servers but our own), so we just see <S:sha1-checksum/> where it names
>> the property that was missing.
>>
>> Basically: a backwards-compat bug where we failed to work against old
>> servers. Fixed. (and no, I won't point fingers at Ivan... oh. oops.
>> :-P )
>>
> Hi Greg,
>
> Thanks for fixing this stupid bug that I introduced.
hehe... no worries, and not stupid. I've done worse :-)
Cheers,
-g
Re: svn commit: r1352044 - /subversion/trunk/subversion/libsvn_ra_serf/update.c
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Wed, Jun 20, 2012 at 3:12 PM, Greg Stein <gs...@gmail.com> wrote:
> Stefan: this should fix your "cat" problem. If the server is old, then
> it reports the property as missing. We don't examine the specific
> status results (tho we should, but then again: we don't talk to any
> servers but our own), so we just see <S:sha1-checksum/> where it names
> the property that was missing.
>
> Basically: a backwards-compat bug where we failed to work against old
> servers. Fixed. (and no, I won't point fingers at Ivan... oh. oops.
> :-P )
>
Hi Greg,
Thanks for fixing this stupid bug that I introduced.
--
Ivan Zhakov
Re: svn commit: r1352044 - /subversion/trunk/subversion/libsvn_ra_serf/update.c
Posted by Greg Stein <gs...@gmail.com>.
On Wed, Jun 20, 2012 at 1:05 PM, Stefan Sperling <st...@elego.de> wrote:
> On Wed, Jun 20, 2012 at 07:12:43AM -0400, Greg Stein wrote:
>> Stefan: this should fix your "cat" problem. If the server is old, then
>> it reports the property as missing. We don't examine the specific
>> status results (tho we should, but then again: we don't talk to any
>> servers but our own), so we just see <S:sha1-checksum/> where it names
>> the property that was missing.
>>
>> Basically: a backwards-compat bug where we failed to work against old
>> servers. Fixed. (and no, I won't point fingers at Ivan... oh. oops.
>> :-P )
>
> Thanks! I did try to debug this last night, and also found that the
> sha1 checksum was an empty string. I wasn't sure though wether we could
> treat NULL and the empty string equivalently. I believe I've seen code
> where NULL indicates properly deletion, but an empty string just sets the
> property to an empty value. Does this idiom not apply to ra_serf?
If you look at the entire XML response, it basically said "404 for the
sha1-checksum property". ra_serf does not examine the associated
status, so we just saw the mention of the property name, which
translates to the empty string. We *should* parse the status to
understand it is saying the property is not present, but that would be
lots of effort for little return.
(but yes: your NULL/empty understanding is correct and ra_serf doesn't
stray from that)
Cheers,
-g
Re: svn commit: r1352044 -
/subversion/trunk/subversion/libsvn_ra_serf/update.c
Posted by Stefan Sperling <st...@elego.de>.
On Wed, Jun 20, 2012 at 07:12:43AM -0400, Greg Stein wrote:
> Stefan: this should fix your "cat" problem. If the server is old, then
> it reports the property as missing. We don't examine the specific
> status results (tho we should, but then again: we don't talk to any
> servers but our own), so we just see <S:sha1-checksum/> where it names
> the property that was missing.
>
> Basically: a backwards-compat bug where we failed to work against old
> servers. Fixed. (and no, I won't point fingers at Ivan... oh. oops.
> :-P )
Thanks! I did try to debug this last night, and also found that the
sha1 checksum was an empty string. I wasn't sure though wether we could
treat NULL and the empty string equivalently. I believe I've seen code
where NULL indicates properly deletion, but an empty string just sets the
property to an empty value. Does this idiom not apply to ra_serf?
Re: svn commit: r1352044 - /subversion/trunk/subversion/libsvn_ra_serf/update.c
Posted by Greg Stein <gs...@gmail.com>.
Stefan: this should fix your "cat" problem. If the server is old, then
it reports the property as missing. We don't examine the specific
status results (tho we should, but then again: we don't talk to any
servers but our own), so we just see <S:sha1-checksum/> where it names
the property that was missing.
Basically: a backwards-compat bug where we failed to work against old
servers. Fixed. (and no, I won't point fingers at Ivan... oh. oops.
:-P )
Cheers,
-g
On Wed, Jun 20, 2012 at 7:08 AM, <gs...@apache.org> wrote:
> Author: gstein
> Date: Wed Jun 20 11:08:45 2012
> New Revision: 1352044
>
> URL: http://svn.apache.org/viewvc?rev=1352044&view=rev
> Log:
> Followup to r1350584: a missing sha1-checksum property can also be the
> empty string (e.g. <S:sha1-checksum/>)
>
> * subversion/libsvn_ra_serf/update.c:
> (try_get_wc_contents): look for both missing and empty string on the
> sha1-checksum property
>
> Modified:
> subversion/trunk/subversion/libsvn_ra_serf/update.c
>
> Modified: subversion/trunk/subversion/libsvn_ra_serf/update.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/update.c?rev=1352044&r1=1352043&r2=1352044&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_ra_serf/update.c (original)
> +++ subversion/trunk/subversion/libsvn_ra_serf/update.c Wed Jun 20 11:08:45 2012
> @@ -2874,9 +2874,9 @@ try_get_wc_contents(svn_boolean_t *found
>
> sha1_checksum_prop = svn_prop_get_value(svn_props, "sha1-checksum");
>
> - if (!sha1_checksum_prop)
> + if (sha1_checksum_prop == NULL || *sha1_checksum_prop == '\0')
> {
> - /* No checksum property in response. */
> + /* No checksum property in response (missing or empty string). */
> return SVN_NO_ERROR;
> }
>
>
>