On Mon, 7 Feb 2005 sussman@tigris.org wrote:
> Modified: branches/locking/subversion/libsvn_ra_dav/fetch.c
> Url: http://svn.collab.net/viewcvs/svn/branches/locking/subversion/libsvn_ra_dav/fetch.c?view=diff&rev=12937&p1=branches/locking/subversion/libsvn_ra_dav/fetch.c&r1=12936&p2=branches/locking/subversion/libsvn_ra_dav/fetch.c&r2=12937
> ==============================================================================
> --- branches/locking/subversion/libsvn_ra_dav/fetch.c (original)
> +++ branches/locking/subversion/libsvn_ra_dav/fetch.c Mon Feb 7 17:20:22 2005
> @@ -1242,6 +1258,319 @@
>
> return err;
> }
...
> +
> +/*
> + * The get-locks-report xml request body is super-simple.
> + * The server doesn't need anything but the URI in the REPORT request line.
> + *
> + * <S:get-locks-report xmlns...>
> + * </S:get-locks-report>
> + *
> + * The get-locks-report xml response is just a list of svn_lock_t's
> + * that exist at or "below" the request URI. (The server runs
> + * svn_repos_fs_get_locks()).
> + *
> + * <S:get-locks-report xmlns...>
> + * <S:lock>
> + * <S:path>/foo/bar/baz</S:path>
> + * <S:token>opaquelocktoken:706689a6-8cef-0310-9809-fb7545cbd44e
> + * </S:token>
> + * <S:owner>fred</S:owner>
> + * <S:comment encoding="base64">ET39IGCB93LL4M</S:comment>
> + * <S:creationdate>2005-02-07 14:17:08 -0600 (Mon, 07 Feb 2005)
> + * </S:creationdate>
I hope you mean
<S:creationdate>2005-02-07T14:17:08Z</S:creationdate>
i.e. not the human date format.
> + * <S:expirationdate>2005-02-08 14:17:08 -0600 (Mon, 08 Feb 2005)
Same.
> + * </S:expirationdate>
> + * </S:lock>
> + * ...
> + * </S:get-locks-report>
> + *
> + *
> + * The <path> and <token> and date-element cdata is xml-escaped by mod_dav_svn.
> + * The <owner> and <comment> cdata is xml-escaped, or *possibly* base64.
It is confusing to say "or *possibly*". It is always XML escaped, per
definition.
> +static int getlocks_end_element(void *userdata, int state,
> + const char *ns, const char *ln)
> +{
> + get_locks_baton_t *baton = userdata;
> + const svn_ra_dav__xml_elm_t *elm;
> + svn_error_t *err;
> +
> + elm = svn_ra_dav__lookup_xml_elem(getlocks_report_elements, ns, ln);
> +
> + /* Just skip unknown elements. */
> + if (elm == NULL)
> + return NE_XML_DECLINE;
> +
> + 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.
> + case ELEM_lock_path:
> + /* neon has already xml-unescaped the cdata for us. */
> + baton->current_lock->path = apr_pstrdup(baton->pool,
> + baton->cdata_accum->data);
> + /* clean up the accumulator. */
> + svn_stringbuf_setempty(baton->cdata_accum);
> + svn_pool_clear(baton->scratchpool);
> + break;
> +
> + case ELEM_lock_token:
> + /* neon has already xml-unescaped the cdata for us. */
> + baton->current_lock->token = apr_pstrdup(baton->pool,
> + baton->cdata_accum->data);
> + /* clean up the accumulator. */
> + svn_stringbuf_setempty(baton->cdata_accum);
> + svn_pool_clear(baton->scratchpool);
> + break;
> +
> + case ELEM_lock_creationdate:
> + err = svn_time_from_cstring(&(baton->current_lock->creation_date),
> + baton->cdata_accum->data,
> + baton->scratchpool);
Yeah, it is the ISO date format.
> + /* clean up the accumulator. */
> + svn_stringbuf_setempty(baton->cdata_accum);
> + svn_pool_clear(baton->scratchpool);
> + break;
> +
> + case ELEM_lock_expirationdate:
> + err = svn_time_from_cstring(&(baton->current_lock->expiration_date),
> + baton->cdata_accum->data,
> + baton->scratchpool);
> + /* clean up the accumulator. */
> + svn_stringbuf_setempty(baton->cdata_accum);
> + svn_pool_clear(baton->scratchpool);
> + break;
> +
> + case ELEM_lock_owner:
> + case ELEM_lock_comment:
> + {
> + 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?
> + 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.
> +
> +
> +
> +svn_error_t *
> +svn_ra_dav__get_locks(svn_ra_session_t *session,
> + apr_hash_t **locks,
> + const char *path,
> + apr_pool_t *pool)
> +{
> + svn_ra_dav__session_t *ras = session->priv;
> + const char *body, *url;
> + svn_error_t *err;
> + int status_code = 0;
> + get_locks_baton_t *baton;
> +
> + baton = apr_pcalloc(pool, sizeof(*baton));
> + baton->lock_hash = apr_hash_make (pool);
> + baton->pool = pool;
> + baton->scratchpool = svn_pool_create(pool);
> + baton->cdata_accum = svn_stringbuf_create("", baton->scratchpool);
> +
> + body = apr_psprintf(pool,
> + "<?xml version=\"1.0\" encoding=\"utf-8\"?>"
> + "<S:get-locks-report xmlns:S=\"" SVN_XML_NAMESPACE "\" "
> + "xmlns:D=\"DAV:\">"
> + "</S:get-locks-report>");
> +
> +
> + /* We always run the report on the 'public' URL, which represents
> + HEAD anyway. If the path doesn't exist in HEAD, then
> + svn_ra_get_locks() *should* fail. Lock queries are always on HEAD. */
> + url = svn_path_url_add_component (ras->url, path, pool);
> +
> + err = svn_ra_dav__parsed_request(ras->sess, "REPORT", url,
> + body, NULL, NULL,
> + getlocks_start_element,
> + getlocks_cdata_handler,
> + getlocks_end_element,
> + baton,
> + NULL, /* extra headers */
> + &status_code,
> + pool);
> +
> + /* Map status 501: Method Not Implemented to our not implemented error.
> + 1.0.x servers and older don't support this report. */
> + if (status_code == 501)
> + return svn_error_create (SVN_ERR_RA_NOT_IMPLEMENTED, err,
> + _("Server does not support locking features."));
TRailing dot:-)
> +
> + if (err && err->apr_err == SVN_ERR_UNSUPPORTED_FEATURE)
> + return svn_error_quick_wrap(err,
> + _("Server does not support locking features"));
> +
> + else if (err)
> + return err;
> +
> + svn_pool_destroy(baton->scratchpool);
> +
> + *locks = baton->lock_hash;
> + return err;
> +}
> +
Best,
//Peter
---------------------------------------------------------------------
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:22:31 2005