* [PATCH] http-push: support full URI in handle_remote_ls_ctx()
2008-12-25 7:04 [PATCH] handle_remote_ls_ctx can parsing href starting at http:// Junio C Hamano
@ 2008-12-29 2:32 ` Kirill A. Korinskiy
2008-12-29 7:17 ` Mike Hommey
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Kirill A. Korinskiy @ 2008-12-29 2:32 UTC (permalink / raw
To: Junio C Hamano; +Cc: git, Kirill A. Korinskiy
The program calls remote_ls() to get list of files from the server
over HTTP; handle_remote_ls_ctx() is used to parse its response to
populate "struct remote_ls_ctx" that is returned from remote_ls().
The handle_remote_ls_ctx() function assumed that the server returns a
local path in href field, but RFC 4918 (14.7) demand of support full
URI (e.g. "http://localhost:8080/repo.git").
This resulted in push failure (e.g. git-http-push issues a PROPFIND
request to "/repo.git/alhost:8080/repo.git/refs/" to the server).
Signed-off-by: Kirill A. Korinskiy <catap@catap.ru>
---
http-push.c | 25 +++++++++++++++++++------
1 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/http-push.c b/http-push.c
index 7c6460919bf3eba10c46cede11ffdd9c53fd2dd2..a4b7d08663504a57008f66a39fffe293f62c1d08 100644
--- a/http-push.c
+++ b/http-push.c
@@ -87,6 +87,7 @@ static struct object_list *objects;
struct repo
{
char *url;
+ char *path;
int path_len;
int has_info_refs;
int can_update_info_refs;
@@ -1424,9 +1425,19 @@ static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)
ls->userFunc(ls);
}
} else if (!strcmp(ctx->name, DAV_PROPFIND_NAME) && ctx->cdata) {
- ls->dentry_name = xmalloc(strlen(ctx->cdata) -
+ char *path = ctx->cdata;
+ if (*ctx->cdata == 'h') {
+ path = strstr(path, "//");
+ if (path) {
+ path = strchr(path+2, '/');
+ }
+ }
+ if (path) {
+ path += remote->path_len;
+ }
+ ls->dentry_name = xmalloc(strlen(path) -
remote->path_len + 1);
- strcpy(ls->dentry_name, ctx->cdata + remote->path_len);
+ strcpy(ls->dentry_name, path + remote->path_len);
} else if (!strcmp(ctx->name, DAV_PROPFIND_COLLECTION)) {
ls->dentry_flags |= IS_DIR;
}
@@ -2206,10 +2217,11 @@ int main(int argc, char **argv)
if (!remote->url) {
char *path = strstr(arg, "//");
remote->url = arg;
+ remote->path_len = strlen(arg);
if (path) {
- path = strchr(path+2, '/');
- if (path)
- remote->path_len = strlen(path);
+ remote->path = strchr(path+2, '/');
+ if (remote->path)
+ remote->path_len = strlen(remote->path);
}
continue;
}
@@ -2238,8 +2250,9 @@ int main(int argc, char **argv)
rewritten_url = xmalloc(strlen(remote->url)+2);
strcpy(rewritten_url, remote->url);
strcat(rewritten_url, "/");
+ remote->path = rewritten_url + (remote->path - remote->url);
+ remote->path_len++;
remote->url = rewritten_url;
- ++remote->path_len;
}
/* Verify DAV compliance/lock support */
--
1.5.6.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] http-push: support full URI in handle_remote_ls_ctx()
2008-12-29 2:32 ` [PATCH] http-push: support full URI in handle_remote_ls_ctx() Kirill A. Korinskiy
@ 2008-12-29 7:17 ` Mike Hommey
2008-12-29 8:36 ` Junio C Hamano
2009-01-02 6:53 ` Kirill A. Korinskiy
2 siblings, 0 replies; 8+ messages in thread
From: Mike Hommey @ 2008-12-29 7:17 UTC (permalink / raw
To: Kirill A. Korinskiy; +Cc: Junio C Hamano, git
On Mon, Dec 29, 2008 at 05:32:15AM +0300, Kirill A. Korinskiy wrote:
> The program calls remote_ls() to get list of files from the server
> over HTTP; handle_remote_ls_ctx() is used to parse its response to
> populate "struct remote_ls_ctx" that is returned from remote_ls().
>
> The handle_remote_ls_ctx() function assumed that the server returns a
> local path in href field, but RFC 4918 (14.7) demand of support full
> URI (e.g. "http://localhost:8080/repo.git").
>
> This resulted in push failure (e.g. git-http-push issues a PROPFIND
> request to "/repo.git/alhost:8080/repo.git/refs/" to the server).
>
> Signed-off-by: Kirill A. Korinskiy <catap@catap.ru>
> ---
> http-push.c | 25 +++++++++++++++++++------
> 1 files changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/http-push.c b/http-push.c
> index 7c6460919bf3eba10c46cede11ffdd9c53fd2dd2..a4b7d08663504a57008f66a39fffe293f62c1d08 100644
> --- a/http-push.c
> +++ b/http-push.c
> @@ -87,6 +87,7 @@ static struct object_list *objects;
> struct repo
> {
> char *url;
> + char *path;
> int path_len;
> int has_info_refs;
> int can_update_info_refs;
> @@ -1424,9 +1425,19 @@ static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)
> ls->userFunc(ls);
> }
> } else if (!strcmp(ctx->name, DAV_PROPFIND_NAME) && ctx->cdata) {
> - ls->dentry_name = xmalloc(strlen(ctx->cdata) -
> + char *path = ctx->cdata;
> + if (*ctx->cdata == 'h') {
> + path = strstr(path, "//");
I would make that a "://"
> + if (path) {
> + path = strchr(path+2, '/');
and s/2/3/, accordingly.
But I realize the existing code already does something like what your
are doing...
Mike
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] http-push: support full URI in handle_remote_ls_ctx()
2008-12-29 2:32 ` [PATCH] http-push: support full URI in handle_remote_ls_ctx() Kirill A. Korinskiy
2008-12-29 7:17 ` Mike Hommey
@ 2008-12-29 8:36 ` Junio C Hamano
2009-01-02 6:53 ` Kirill A. Korinskiy
2 siblings, 0 replies; 8+ messages in thread
From: Junio C Hamano @ 2008-12-29 8:36 UTC (permalink / raw
To: Kirill A. Korinskiy; +Cc: git
Thanks; already applied ;-)
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] http-push: support full URI in handle_remote_ls_ctx()
2008-12-29 2:32 ` [PATCH] http-push: support full URI in handle_remote_ls_ctx() Kirill A. Korinskiy
2008-12-29 7:17 ` Mike Hommey
2008-12-29 8:36 ` Junio C Hamano
@ 2009-01-02 6:53 ` Kirill A. Korinskiy
2009-01-02 7:26 ` Junio C Hamano
2 siblings, 1 reply; 8+ messages in thread
From: Kirill A. Korinskiy @ 2009-01-02 6:53 UTC (permalink / raw
To: Junio C Hamano; +Cc: git, Kirill A. Korinskiy
The program calls remote_ls() to get list of files from the server
over HTTP; handle_remote_ls_ctx() is used to parse its response to
populate "struct remote_ls_ctx" that is returned from remote_ls().
The handle_remote_ls_ctx() function assumed that the server returns a
local path in href field, but RFC 4918 (14.7) demand of support full
URI (e.g. "http://localhost:8080/repo.git").
This resulted in push failure (e.g. git-http-push issues a PROPFIND
request to "/repo.git/alhost:8080/repo.git/refs/" to the server).
Signed-off-by: Kirill A. Korinskiy <catap@catap.ru>
---
http-push.c | 25 +++++++++++++++++++------
1 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/http-push.c b/http-push.c
index 7c6460919bf3eba10c46cede11ffdd9c53fd2dd2..d1749fe2ffd6a59a4eea514997a62b5d7c80b438 100644
--- a/http-push.c
+++ b/http-push.c
@@ -87,6 +87,7 @@ static struct object_list *objects;
struct repo
{
char *url;
+ char *path;
int path_len;
int has_info_refs;
int can_update_info_refs;
@@ -1424,9 +1425,19 @@ static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)
ls->userFunc(ls);
}
} else if (!strcmp(ctx->name, DAV_PROPFIND_NAME) && ctx->cdata) {
- ls->dentry_name = xmalloc(strlen(ctx->cdata) -
+ char *path = ctx->cdata;
+ if (*ctx->cdata == 'h') {
+ path = strstr(path, "://");
+ if (path) {
+ path = strchr(path+3, '/');
+ }
+ }
+ if (path) {
+ path += remote->path_len;
+ }
+ ls->dentry_name = xmalloc(strlen(path) -
remote->path_len + 1);
- strcpy(ls->dentry_name, ctx->cdata + remote->path_len);
+ strcpy(ls->dentry_name, path + remote->path_len);
} else if (!strcmp(ctx->name, DAV_PROPFIND_COLLECTION)) {
ls->dentry_flags |= IS_DIR;
}
@@ -2206,10 +2217,11 @@ int main(int argc, char **argv)
if (!remote->url) {
char *path = strstr(arg, "//");
remote->url = arg;
+ remote->path_len = strlen(arg);
if (path) {
- path = strchr(path+2, '/');
- if (path)
- remote->path_len = strlen(path);
+ remote->path = strchr(path+2, '/');
+ if (remote->path)
+ remote->path_len = strlen(remote->path);
}
continue;
}
@@ -2238,8 +2250,9 @@ int main(int argc, char **argv)
rewritten_url = xmalloc(strlen(remote->url)+2);
strcpy(rewritten_url, remote->url);
strcat(rewritten_url, "/");
+ remote->path = rewritten_url + (remote->path - remote->url);
+ remote->path_len++;
remote->url = rewritten_url;
- ++remote->path_len;
}
/* Verify DAV compliance/lock support */
--
1.5.6.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] http-push: support full URI in handle_remote_ls_ctx()
2009-01-02 6:53 ` Kirill A. Korinskiy
@ 2009-01-02 7:26 ` Junio C Hamano
0 siblings, 0 replies; 8+ messages in thread
From: Junio C Hamano @ 2009-01-02 7:26 UTC (permalink / raw
To: Kirill A. Korinskiy; +Cc: git
"Kirill A. Korinskiy" <catap@catap.ru> writes:
> @@ -1424,9 +1425,19 @@ static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)
> ls->userFunc(ls);
> }
> } else if (!strcmp(ctx->name, DAV_PROPFIND_NAME) && ctx->cdata) {
> - ls->dentry_name = xmalloc(strlen(ctx->cdata) -
> + char *path = ctx->cdata;
> + if (*ctx->cdata == 'h') {
> + path = strstr(path, "://");
> + if (path) {
> + path = strchr(path+3, '/');
> + }
> + }
Is this "://" (and +3) the only change from the previous one that has
already been queued? I didn't have a problem with the old "//" one.
The check to see if it begins with 'h' bothers me much much more.
If you want to be defensively tight, you should be checking if it begins
with either "http://" or "https://", the only two protocols you are
prepared to handle, and nothing else, so that you won't trigger this
codepath when the other end gave you "hqrt://..", on the basis that your
code won't know if hqrt:// protocol works the same way as http and https.
On the other hand, if you want to be optimistically loose, expecting
whatever people would implement that can be handled with the existing DAV
code would behave the same way as http and https, you shouldn't be
limiting yourself to an unknown protocol name that happens to begin with
an 'h', only accepting "hqrt://" but not "ittp://" URLs.
Your "first byte of the protocol name must be 'h'" does not do either.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] http-push: support full URI in handle_remote_ls_ctx()
@ 2009-01-18 11:28 Kirill A. Korinskiy
2009-01-18 14:51 ` Johannes Schindelin
2009-01-18 21:28 ` Junio C Hamano
0 siblings, 2 replies; 8+ messages in thread
From: Kirill A. Korinskiy @ 2009-01-18 11:28 UTC (permalink / raw
To: Junio C Hamano; +Cc: git, Kirill A. Korinskiy
The program calls remote_ls() to get list of files from the server
over HTTP; handle_remote_ls_ctx() is used to parse its response to
populate "struct remote_ls_ctx" that is returned from remote_ls().
The handle_remote_ls_ctx() function assumed that the server returns a
local path in href field, but RFC 4918 (14.7) demand of support full
URI (e.g. "http://localhost:8080/repo.git").
This resulted in push failure (e.g. git-http-push issues a PROPFIND
request to "/repo.git/alhost:8080/repo.git/refs/" to the server).
Signed-off-by: Kirill A. Korinskiy <catap@catap.ru>
---
http-push.c | 24 ++++++++++++++++++------
1 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/http-push.c b/http-push.c
index 7c6460919bf3eba10c46cede11ffdd9c53fd2dd2..2cb925a9ad857b6d79858d5187f14072167282e7 100644
--- a/http-push.c
+++ b/http-push.c
@@ -87,6 +87,7 @@ static struct object_list *objects;
struct repo
{
char *url;
+ char *path;
int path_len;
int has_info_refs;
int can_update_info_refs;
@@ -1424,9 +1425,18 @@ static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)
ls->userFunc(ls);
}
} else if (!strcmp(ctx->name, DAV_PROPFIND_NAME) && ctx->cdata) {
- ls->dentry_name = xmalloc(strlen(ctx->cdata) -
+ char *path = ctx->cdata;
+ if (!strcmp(ctx->cdata, "http://")) {
+ path = strchr(path + sizeof("http://") - 1, '/');
+ } else if (!strcmp(ctx->cdata, "https://")) {
+ path = strchr(path + sizeof("https://") - 1, '/');
+ }
+
+ path += remote->path_len;
+
+ ls->dentry_name = xmalloc(strlen(path) -
remote->path_len + 1);
- strcpy(ls->dentry_name, ctx->cdata + remote->path_len);
+ strcpy(ls->dentry_name, path + remote->path_len);
} else if (!strcmp(ctx->name, DAV_PROPFIND_COLLECTION)) {
ls->dentry_flags |= IS_DIR;
}
@@ -2206,10 +2216,11 @@ int main(int argc, char **argv)
if (!remote->url) {
char *path = strstr(arg, "//");
remote->url = arg;
+ remote->path_len = strlen(arg);
if (path) {
- path = strchr(path+2, '/');
- if (path)
- remote->path_len = strlen(path);
+ remote->path = strchr(path+2, '/');
+ if (remote->path)
+ remote->path_len = strlen(remote->path);
}
continue;
}
@@ -2238,8 +2249,9 @@ int main(int argc, char **argv)
rewritten_url = xmalloc(strlen(remote->url)+2);
strcpy(rewritten_url, remote->url);
strcat(rewritten_url, "/");
+ remote->path = rewritten_url + (remote->path - remote->url);
+ remote->path_len++;
remote->url = rewritten_url;
- ++remote->path_len;
}
/* Verify DAV compliance/lock support */
--
1.5.6.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] http-push: support full URI in handle_remote_ls_ctx()
2009-01-18 11:28 [PATCH] http-push: support full URI in handle_remote_ls_ctx() Kirill A. Korinskiy
@ 2009-01-18 14:51 ` Johannes Schindelin
2009-01-18 21:28 ` Junio C Hamano
1 sibling, 0 replies; 8+ messages in thread
From: Johannes Schindelin @ 2009-01-18 14:51 UTC (permalink / raw
To: Kirill A. Korinskiy; +Cc: Junio C Hamano, git
Hi,
On Sun, 18 Jan 2009, Kirill A. Korinskiy wrote:
> The program calls remote_ls() to get list of files from the server
> over HTTP; handle_remote_ls_ctx() is used to parse its response to
> populate "struct remote_ls_ctx" that is returned from remote_ls().
>
> The handle_remote_ls_ctx() function assumed that the server returns a
> local path in href field, but RFC 4918 (14.7) demand of support full
> URI (e.g. "http://localhost:8080/repo.git").
>
> This resulted in push failure (e.g. git-http-push issues a PROPFIND
> request to "/repo.git/alhost:8080/repo.git/refs/" to the server).
>
> Signed-off-by: Kirill A. Korinskiy <catap@catap.ru>
> ---
> http-push.c | 24 ++++++++++++++++++------
> 1 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/http-push.c b/http-push.c
> index 7c6460919bf3eba10c46cede11ffdd9c53fd2dd2..2cb925a9ad857b6d79858d5187f14072167282e7 100644
You mean this patch is not vs
junio/next(bbe7a8ed3dac72b7a1372cd92f68f47965c10100) or junio/master or
junio/maint(both a4b7d08663504a57008f66a39fffe293f62c1d08) but
tags/v1.6.1-rc4~13?
Ciao,
Dscho
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] http-push: support full URI in handle_remote_ls_ctx()
2009-01-18 11:28 [PATCH] http-push: support full URI in handle_remote_ls_ctx() Kirill A. Korinskiy
2009-01-18 14:51 ` Johannes Schindelin
@ 2009-01-18 21:28 ` Junio C Hamano
1 sibling, 0 replies; 8+ messages in thread
From: Junio C Hamano @ 2009-01-18 21:28 UTC (permalink / raw
To: Kirill A. Korinskiy; +Cc: git
"Kirill A. Korinskiy" <catap@catap.ru> writes:
> @@ -1424,9 +1425,18 @@ static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)
> ls->userFunc(ls);
> }
> } else if (!strcmp(ctx->name, DAV_PROPFIND_NAME) && ctx->cdata) {
> - ls->dentry_name = xmalloc(strlen(ctx->cdata) -
> + char *path = ctx->cdata;
> + if (!strcmp(ctx->cdata, "http://")) {
> + path = strchr(path + sizeof("http://") - 1, '/');
> + } else if (!strcmp(ctx->cdata, "https://")) {
> + path = strchr(path + sizeof("https://") - 1, '/');
> + }
> +
> + path += remote->path_len;
I see you chose to address the issue I pointed out in:
http://thread.gmane.org/gmane.comp.version-control.git/103804/focus=104363
by being more strict. That's the only change I can spot compared to
e1f33ef (http-push: support full URI in handle_remote_ls_ctx(),
2008-12-23) that is already in maint.
Could you make this into an incremental patch?
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2009-01-18 21:29 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-01-18 11:28 [PATCH] http-push: support full URI in handle_remote_ls_ctx() Kirill A. Korinskiy
2009-01-18 14:51 ` Johannes Schindelin
2009-01-18 21:28 ` Junio C Hamano
-- strict thread matches above, loose matches on Subject: below --
2008-12-25 7:04 [PATCH] handle_remote_ls_ctx can parsing href starting at http:// Junio C Hamano
2008-12-29 2:32 ` [PATCH] http-push: support full URI in handle_remote_ls_ctx() Kirill A. Korinskiy
2008-12-29 7:17 ` Mike Hommey
2008-12-29 8:36 ` Junio C Hamano
2009-01-02 6:53 ` Kirill A. Korinskiy
2009-01-02 7:26 ` Junio C Hamano
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.