[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r12937 - branches/locking/subversion/libsvn_ra_dav

From: Ben Collins-Sussman <sussman_at_collab.net>
Date: 2005-02-08 15:35:51 CET

On Feb 8, 2005, at 8:22 AM, Peter N. Lundblad wrote:
>>
>> + switch (elm->id)
>> + {
>> + case ELEM_lock:
>> + /* finish the whole svn_lock_t. */
>> + apr_hash_set(baton->lock_hash, baton->current_lock->path,
>> + APR_HASH_KEY_STRING, baton->current_lock);
>> + break;
>> +
> I think you want to validate the lock here. Else, misbehaving servers
> can
> crash the client. This means checking that mandatory fields really were
> set.

Hm, maybe we should create some svn_lock_validate() function in
libsvn_subr? I imagine that other RA layers might want to validate
locks coming from the network too, yes?

>> const char *final_val;
>> +
>> + if (baton->encoding)
>> + {
>> + /* ### possibly recognize other encodings someday */
>
> What other encodings would that be? Wouldn't it be simple to just use a
> boolean in the baton for base64?

kfogel and cmpilato wanted to leave the doors open for other possible
encodings in the future. Nothing specific in mind, just wanted to keep
it flexible.

>
>> + if (strcmp(baton->encoding, "base64") == 0)
>> + {
>> + svn_string_t *encoded_val;
>> + const svn_string_t *decoded_val;
>> +
>> + encoded_val =
>> svn_string_create_from_buf(baton->cdata_accum,
>> +
>> baton->scratchpool);
>> + decoded_val = svn_base64_decode_string(encoded_val,
>> +
>> baton->scratchpool);
>> + final_val = decoded_val->data;
>> + }
>> + else
>> + /* unrecognized encoding! */
>> + return NE_XML_ABORT;
>> +
>> + baton->encoding = NULL;
>> + }
>> + else
>> + {
>> + /* neon has already xml-unescaped the cdata for us. */
>> + final_val = baton->cdata_accum->data;
>> + }
>> +
>> + if (elm->id == ELEM_lock_owner)
>> + baton->current_lock->owner = apr_pstrdup(baton->pool,
>> final_val);
>> + if (elm->id == ELEM_lock_comment)
>> + baton->current_lock->comment = apr_pstrdup(baton->pool,
>> final_val);
>> +
>> + /* clean up the accumulator. */
>> + svn_stringbuf_setempty(baton->cdata_accum);
>> + svn_pool_clear(baton->scratchpool);
>> + break;
>> + }
>> +
>> +
>> + default:
>> + break;
>> + }
>> +
>> + return 0;
>> +}
>
> err is leaked. I think you need to store the error in the baton and
> return
> it after the parse.

What err??

I'll fix the rest of this stuff... thanks!

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Feb 8 15:38:09 2005

This is an archived mail posted to the Subversion Dev mailing list.