threads / patch / 16791

patchRemove the requirement opaquelocktoken uri scheme

Subject: [PATCH] Remove the requirement opaquelocktoken uri scheme

## tl;dr

4 messages between Dec 19, 2008 and Dec 20, 2008. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Kirill A. Korinskiy· Dec 19, 2008, 01:51 UTC · lore

Server can use any URI for token by rfc 4918 section 6.5 paragraph five

Signed-off-by: Kirill A. Korinskiy <catap@catap.ru>
---
 http-push.c |   14 ++++++--------
 1 files changed, 6 insertions(+), 8 deletions(-)

Show changes to http-push.c +6 −8

diff --git a/http-push.c b/http-push.c
index 5cecef434a7740a3f853462978c3e071b4da7e74..7c6460919bf3eba10c46cede11ffdd9c53fd2dd2 100644
--- a/http-push.c
+++ b/http-push.c
@@ -595,7 +595,7 @@ static int refresh_lock(struct remote_lock *lock)
 	lock->refreshing = 1;
 
 	if_header = xmalloc(strlen(lock->token) + 25);
-	sprintf(if_header, "If: (<opaquelocktoken:%s>)", lock->token);
+	sprintf(if_header, "If: (<%s>)", lock->token);
 	sprintf(timeout_header, "Timeout: Second-%ld", lock->timeout);
 	dav_headers = curl_slist_append(dav_headers, if_header);
 	dav_headers = curl_slist_append(dav_headers, timeout_header);
@@ -1120,10 +1120,8 @@ static void handle_new_lock_ctx(struct xml_ctx *ctx, int tag_closed)
 				lock->timeout =
 					strtol(ctx->cdata + 7, NULL, 10);
 		} else if (!strcmp(ctx->name, DAV_ACTIVELOCK_TOKEN)) {
-			if (!prefixcmp(ctx->cdata, "opaquelocktoken:")) {
-				lock->token = xmalloc(strlen(ctx->cdata) - 15);
-				strcpy(lock->token, ctx->cdata + 16);
-			}
+			lock->token = xmalloc(strlen(ctx->cdata) + 1);
+			strcpy(lock->token, ctx->cdata);
 		}
 	}
 }
@@ -1308,7 +1306,7 @@ static int unlock_remote(struct remote_lock *lock)
 	int rc = 0;
 
 	lock_token_header = xmalloc(strlen(lock->token) + 31);
-	sprintf(lock_token_header, "Lock-Token: <opaquelocktoken:%s>",
+	sprintf(lock_token_header, "Lock-Token: <%s>",
 		lock->token);
 	dav_headers = curl_slist_append(dav_headers, lock_token_header);
 
@@ -1722,7 +1720,7 @@ static int update_remote(unsigned char *sha1, struct remote_lock *lock)
 	struct curl_slist *dav_headers = NULL;
 
 	if_header = xmalloc(strlen(lock->token) + 25);
-	sprintf(if_header, "If: (<opaquelocktoken:%s>)", lock->token);
+	sprintf(if_header, "If: (<%s>)", lock->token);
 	dav_headers = curl_slist_append(dav_headers, if_header);
 
 	strbuf_addf(&out_buffer.buf, "%s\n", sha1_to_hex(sha1));
@@ -1941,7 +1939,7 @@ static void update_remote_info_refs(struct remote_lock *lock)
 		  add_remote_info_ref, &buffer.buf);
 	if (!aborted) {
 		if_header = xmalloc(strlen(lock->token) + 25);
-		sprintf(if_header, "If: (<opaquelocktoken:%s>)", lock->token);
+		sprintf(if_header, "If: (<%s>)", lock->token);
 		dav_headers = curl_slist_append(dav_headers, if_header);
 
 		slot = get_active_slot();

-- 
1.5.6.5

Junio C Hamano· Dec 19, 2008, 07:32 UTC · re: Kirill A. Korinskiy · lore

Re: [PATCH] Remove the requirement opaquelocktoken uri scheme

"Kirill A. Korinskiy" <catap@catap.ru> writes:

> Server can use any URI for token by rfc 4918 section 6.5 paragraph five
>
> Signed-off-by: Kirill A. Korinskiy <catap@catap.ru>

Could you give a bit more high-level background information behind this patch?

I can make a guess without knowing much about DAV that this might be...

  The program flow of pushing over http is:

   - call lock_remote() to issue a DAV_LOCK request to the server to lock
     info/refs and branch refs being pushed into; handle_new_lock_ctx() is
     used to parse its response to populate "struct remote_lock" that is
     returned from lock_remote();

   - send objects;

   - call unlock_remote() to drop the lock.

  The handle_new_lock_ctx() function assumed that the server will use a
  lock token in opaquelocktoken URI scheme, which may have been an Ok
  assumption under RFC 2518, but under RFC 4918 which obsoletes the older
  standard it is not necessarily true.

  This resulted in push failure (often resulted in "xxxxx" error message)
  when talking to a server that does not use opaquelocktoken URI scheme.

But I shouldn't have to guess or write the commit log message for you.

Giving a bit higher level background is important for people who may have seen the error message (so filling in the "xxxxx" blank in the above hypothetical commit log message is *important*) to find your message and try your commit to see if it fixes the issue for them.

Kirill A. Korinskiy· Dec 20, 2008, 06:19 UTC · re: Junio C Hamano · lore

The program flow of pushing over http is:

 - call lock_remote() to issue a DAV_LOCK request to the server to lock
   info/refs and branch refs being pushed into; handle_new_lock_ctx() is
   used to parse its response to populate "struct remote_lock" that is
   returned from lock_remote();

 - send objects;

 - call unlock_remote() to drop the lock.

The handle_new_lock_ctx() function assumed that the server will use a lock token in opaquelocktoken URI scheme, which may have been an Ok assumption under RFC 2518, but under RFC 4918 which obsoletes the older standard it is not necessarily true.

This resulted in push failure (often resulted in "cannot lock existing info/refs" error message) when talking to a server that does not use opaquelocktoken URI scheme.

Signed-off-by: Kirill A. Korinskiy <catap@catap.ru>
---
 http-push.c |   14 ++++++--------
 1 files changed, 6 insertions(+), 8 deletions(-)

Show changes to http-push.c +6 −8

diff --git a/http-push.c b/http-push.c
index 5cecef434a7740a3f853462978c3e071b4da7e74..7c6460919bf3eba10c46cede11ffdd9c53fd2dd2 100644
--- a/http-push.c
+++ b/http-push.c
@@ -595,7 +595,7 @@ static int refresh_lock(struct remote_lock *lock)
 	lock->refreshing = 1;
 
 	if_header = xmalloc(strlen(lock->token) + 25);
-	sprintf(if_header, "If: (<opaquelocktoken:%s>)", lock->token);
+	sprintf(if_header, "If: (<%s>)", lock->token);
 	sprintf(timeout_header, "Timeout: Second-%ld", lock->timeout);
 	dav_headers = curl_slist_append(dav_headers, if_header);
 	dav_headers = curl_slist_append(dav_headers, timeout_header);
@@ -1120,10 +1120,8 @@ static void handle_new_lock_ctx(struct xml_ctx *ctx, int tag_closed)
 				lock->timeout =
 					strtol(ctx->cdata + 7, NULL, 10);
 		} else if (!strcmp(ctx->name, DAV_ACTIVELOCK_TOKEN)) {
-			if (!prefixcmp(ctx->cdata, "opaquelocktoken:")) {
-				lock->token = xmalloc(strlen(ctx->cdata) - 15);
-				strcpy(lock->token, ctx->cdata + 16);
-			}
+			lock->token = xmalloc(strlen(ctx->cdata) + 1);
+			strcpy(lock->token, ctx->cdata);
 		}
 	}
 }
@@ -1308,7 +1306,7 @@ static int unlock_remote(struct remote_lock *lock)
 	int rc = 0;
 
 	lock_token_header = xmalloc(strlen(lock->token) + 31);
-	sprintf(lock_token_header, "Lock-Token: <opaquelocktoken:%s>",
+	sprintf(lock_token_header, "Lock-Token: <%s>",
 		lock->token);
 	dav_headers = curl_slist_append(dav_headers, lock_token_header);
 
@@ -1722,7 +1720,7 @@ static int update_remote(unsigned char *sha1, struct remote_lock *lock)
 	struct curl_slist *dav_headers = NULL;
 
 	if_header = xmalloc(strlen(lock->token) + 25);
-	sprintf(if_header, "If: (<opaquelocktoken:%s>)", lock->token);
+	sprintf(if_header, "If: (<%s>)", lock->token);
 	dav_headers = curl_slist_append(dav_headers, if_header);
 
 	strbuf_addf(&out_buffer.buf, "%s\n", sha1_to_hex(sha1));
@@ -1941,7 +1939,7 @@ static void update_remote_info_refs(struct remote_lock *lock)
 		  add_remote_info_ref, &buffer.buf);
 	if (!aborted) {
 		if_header = xmalloc(strlen(lock->token) + 25);
-		sprintf(if_header, "If: (<opaquelocktoken:%s>)", lock->token);
+		sprintf(if_header, "If: (<%s>)", lock->token);
 		dav_headers = curl_slist_append(dav_headers, if_header);
 
 		slot = get_active_slot();

-- 
1.5.6.5

Junio C Hamano· Dec 20, 2008, 20:09 UTC · re: Kirill A. Korinskiy · lore

Re: [PATCH] Remove the requirement opaquelocktoken uri scheme

"Kirill A. Korinskiy" <catap@catap.ru> writes:

Show 6 quoted lines

> ...
> This resulted in push failure (often resulted in "cannot lock existing
> info/refs" error message) when talking to a server that does not use
> opaquelocktoken URI scheme.
>
> Signed-off-by: Kirill A. Korinskiy <catap@catap.ru>

Thanks.