[PATCH] use lock token in non-URI form in start_put

After 753bc91 ("Remove the requirement opaquelocktoken uri scheme"),
lock tokens are in the URI forms in which they are received from the
server, eg. 'opaquelocktoken:', 'uuid:'

However, "start_put" (and consequently "start_move"), which attempts to
create a unique temporary file using the UUID of the lock token,
inadvertently uses the lock token in its URI form. These file
operations on the server may not be successful (specifically, in
Windows), due to the colon ':' character from the URI form of the lock
token in the file path.

This patch ensures that the lock token sans the URI scheme is used
instead in "start_put".

To do this, the "start_put" gets the position of ':', which is used to
separate the URI scheme from the part, eg. "<scheme>:". In addition,
start_put uses the last position of ':', since URIs with component
URIs are possible, eg. "urn:uuid:" One can be sure that the lock token
will always contain the UUID and be in URI form, due to RFC 2518, or
its successor RFC 4918 (see
http://www.webdav.org/specs/rfc4918.html#ELEMENT_locktoken).

Signed-off-by: Tay Ray Chuan <redacted>

Signed-off-by: Tay Ray Chuan <redacted>
---
 http-push.c          |    2 +-
 t/t5540-http-push.sh |    7 +++++++
 2 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/http-push.c b/http-push.c
index eefd64c..bd8f372 100644
--- a/http-push.c
+++ b/http-push.c

@@ -558,7 +558,7 @@ static void start_put(struct transfer_request *request)

 	append_remote_object_url(&buf, remote->url, hex, 0);
 	strbuf_addstr(&buf, "_");
-	strbuf_addstr(&buf, request->lock->token);
+	strbuf_addstr(&buf, strrchr(request->lock->token, ':') + 1);
 	request->url = strbuf_detach(&buf, NULL);

 	slot = get_active_slot();

diff --git a/t/t5540-http-push.sh b/t/t5540-http-push.sh
index c236b5e..268b2d4 100755
--- a/t/t5540-http-push.sh
+++ b/t/t5540-http-push.sh

@@ -94,6 +94,13 @@ test_expect_success 'MKCOL sends directory names with trailing slashes' '

 '

+test_expect_success 'PUT and MOVE sends object to URLs in non-URI form' '
+
+	grep -P "\"(?:PUT|MOVE) .+objects/[\da-z]{2}/[\da-z]{38}_[\da-z\-]{36} HTTP/[0-9.]+\" 20\d" \
+	< "$HTTPD_ROOT_PATH"/access.log
+
+'
+
 stop_httpd

 test_done

-- 
1.6.1.2.278.g9a9e.dirty

Hi,

On Sun, 8 Feb 2009, Tay Ray Chuan wrote:

After 753bc91 ("Remove the requirement opaquelocktoken uri scheme"),
lock tokens are in the URI forms in which they are received from the
server, eg. 'opaquelocktoken:', 'uuid:'

However, "start_put" (and consequently "start_move"), which attempts to
create a unique temporary file using the UUID of the lock token,
inadvertently uses the lock token in its URI form. These file
operations on the server may not be successful (specifically, in
Windows), due to the colon ':' character from the URI form of the lock
token in the file path.

If it is a prefix that happens to be part of the URI, but must not be used 
by the client code as a lock token, would it not be better to store the 
token in lock->token to begin with?

This is unsafe.  What if lock->token does not contain a colon?  Even if it 
happens to be the case now, in your setup, it might change, or there might 
be mistakes in the server code.  We should always play it safe if we 
cannot control the other side's code.

Ciao,
Dscho

Hi,

On Sun, Feb 8, 2009 at 4:20 AM, Johannes Schindelin
[off-list ref] wrote:

If it is a prefix that happens to be part of the URI, but must not be used
by the client code as a lock token, would it not be better to store the
token in lock->token to begin with?

The URI form of the lock token is suitable for all other occurrences
lock token usage; that is, only start_put needs the non-URI form,
while the rest use the URI form.

That's why I only changed how start_put uses the lock token; changing
lock token just for the sake of start_put doesn't seem very effective
to me.

This is unsafe.  What if lock->token does not contain a colon?  Even if it
happens to be the case now, in your setup, it might change, or there might
be mistakes in the server code.  We should always play it safe if we
cannot control the other side's code.

Point noted. I'll try to think of something else.

-- 
Cheers,
Ray Chuan