Re: [PATCH] Verify Content-Type from smart HTTP servers

Subsystems: the rest

3 messages, 3 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] Verify Content-Type from smart HTTP servers

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:56:02

Jeff King [off-list ref] writes:
quoted hunk
I was specifically thinking of this (on top of your patch):
diff --git a/remote-curl.c b/remote-curl.c
index e6f3b63..63680a8 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -134,14 +134,12 @@ static struct discovery* discover_refs(const char *service)
 	last->buf_alloc = strbuf_detach(&buffer, &last->len);
 	last->buf = last->buf_alloc;
 
-	if (maybe_smart && 5 <= last->len && last->buf[4] == '#') {
+	strbuf_addf(&exp, "application/x-%s-advertisement", service);
+	if (maybe_smart && !strbuf_cmp(&exp, &type)) {
 		/*
 		 * smart HTTP response; validate that the service
 		 * pkt-line matches our request.
 		 */
-		strbuf_addf(&exp, "application/x-%s-advertisement", service);
-		if (strbuf_cmp(&exp, &type))
-			die("invalid content-type %s", type.buf);
 		if (packet_get_line(&buffer, &last->buf, &last->len) <= 0)
 			die("%s has invalid packet header", refs_url);
 		if (buffer.len && buffer.buf[buffer.len - 1] == '\n')
To just follow the dumb path if we don't get the content-type we expect.
We may want to keep the '#' format check in addition (packet_get_line
will check it and die, anyway, but we may want to drop back to
considering it dumb, just to protect against a badly configured dumb
server which uses our mime type, but I do not think it likely).
Yeah, but it doesn't cost anything to check, so let's do so.

Does this look good to both of you (relative to Shawn's patch)?

 remote-curl.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/remote-curl.c b/remote-curl.c
index e6f3b63..933c69a 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -134,14 +134,14 @@ static struct discovery* discover_refs(const char *service)
 	last->buf_alloc = strbuf_detach(&buffer, &last->len);
 	last->buf = last->buf_alloc;
 
-	if (maybe_smart && 5 <= last->len && last->buf[4] == '#') {
+	strbuf_addf(&exp, "application/x-%s-advertisement", service);
+	if (maybe_smart &&
+	    (5 <= last->len && last->buf[4] == '#') &&
+	    !strbuf_cmp(&exp, &type)) {
 		/*
 		 * smart HTTP response; validate that the service
 		 * pkt-line matches our request.
 		 */
-		strbuf_addf(&exp, "application/x-%s-advertisement", service);
-		if (strbuf_cmp(&exp, &type))
-			die("invalid content-type %s", type.buf);
 		if (packet_get_line(&buffer, &last->buf, &last->len) <= 0)
 			die("%s has invalid packet header", refs_url);
 		if (buffer.len && buffer.buf[buffer.len - 1] == '\n')

Re: [PATCH] Verify Content-Type from smart HTTP servers

From: Jeff King <hidden>
Date: 2016-06-15 22:56:02

On Sun, Feb 03, 2013 at 11:17:33PM -0800, Junio C Hamano wrote:
quoted hunk
Does this look good to both of you (relative to Shawn's patch)?

 remote-curl.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/remote-curl.c b/remote-curl.c
index e6f3b63..933c69a 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -134,14 +134,14 @@ static struct discovery* discover_refs(const char *service)
 	last->buf_alloc = strbuf_detach(&buffer, &last->len);
 	last->buf = last->buf_alloc;
 
-	if (maybe_smart && 5 <= last->len && last->buf[4] == '#') {
+	strbuf_addf(&exp, "application/x-%s-advertisement", service);
+	if (maybe_smart &&
+	    (5 <= last->len && last->buf[4] == '#') &&
+	    !strbuf_cmp(&exp, &type)) {
 		/*
 		 * smart HTTP response; validate that the service
 		 * pkt-line matches our request.
 		 */
-		strbuf_addf(&exp, "application/x-%s-advertisement", service);
-		if (strbuf_cmp(&exp, &type))
-			die("invalid content-type %s", type.buf);
 		if (packet_get_line(&buffer, &last->buf, &last->len) <= 0)
 			die("%s has invalid packet header", refs_url);
 		if (buffer.len && buffer.buf[buffer.len - 1] == '\n')
Yeah, I think that's fine. Thanks.

-Peff

Re: [PATCH] Verify Content-Type from smart HTTP servers

From: Shawn Pearce <hidden>
Date: 2016-06-15 22:56:03

On Mon, Feb 4, 2013 at 12:38 AM, Jeff King [off-list ref] wrote:
On Sun, Feb 03, 2013 at 11:17:33PM -0800, Junio C Hamano wrote:
quoted
Does this look good to both of you (relative to Shawn's patch)?

 remote-curl.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/remote-curl.c b/remote-curl.c
index e6f3b63..933c69a 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -134,14 +134,14 @@ static struct discovery* discover_refs(const char *service)
      last->buf_alloc = strbuf_detach(&buffer, &last->len);
      last->buf = last->buf_alloc;

-     if (maybe_smart && 5 <= last->len && last->buf[4] == '#') {
+     strbuf_addf(&exp, "application/x-%s-advertisement", service);
+     if (maybe_smart &&
+         (5 <= last->len && last->buf[4] == '#') &&
+         !strbuf_cmp(&exp, &type)) {
              /*
               * smart HTTP response; validate that the service
               * pkt-line matches our request.
               */
-             strbuf_addf(&exp, "application/x-%s-advertisement", service);
-             if (strbuf_cmp(&exp, &type))
-                     die("invalid content-type %s", type.buf);
              if (packet_get_line(&buffer, &last->buf, &last->len) <= 0)
                      die("%s has invalid packet header", refs_url);
              if (buffer.len && buffer.buf[buffer.len - 1] == '\n')
Yeah, I think that's fine. Thanks.
Looks fine to me too, but I think the test won't work now. :-)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help