Re: [PATCH] http-backend: respect GIT_NAMESPACE with dumb clients

[Josh Triplett <josh@joshtriplett.org>]

On Wed, Mar 27, 2013 at 11:02:28PM -0700, Junio C Hamano wrote:
> John Koleszar <jkoleszar@google.com> writes:
> 
> > Filter the list of refs returned via the dumb HTTP protocol according
> > to the active namespace, consistent with other clients of the
> > upload-pack service.
> >
> > Signed-off-by: John Koleszar <jkoleszar@google.com>
> > ---
> 
> Looks sane from a cursory read---thanks.
> 
> Josh, any comments?

Looks reasonable; glad to see tests explicitly covering it, as well.

Ideally I'd like to see an additional test against that same namespaced
repository, fetching from a different URL that doesn't set
GIT_NAMESPACE, and verifying that info/refs contains the full set of
namespace-qualified refs from all namespaces.  That would make sure that
particular behavior doesn't regress in the future, since one of the use
cases for namespaced repositories involves fetching the whole combined
repo, for purposes such as backups or replication.

> >  http-backend.c          |  8 +++++---
> >  t/lib-httpd/apache.conf |  5 +++++
> >  t/t5561-http-backend.sh |  4 ++++
> >  t/t556x_common          | 16 ++++++++++++++++
> >  4 files changed, 30 insertions(+), 3 deletions(-)
> >
> > diff --git a/http-backend.c b/http-backend.c
> > index f50e77f..b9896b0 100644
> > --- a/http-backend.c
> > +++ b/http-backend.c
> > @@ -361,17 +361,19 @@ static void run_service(const char **argv)
> >  static int show_text_ref(const char *name, const unsigned char *sha1,
> >  	int flag, void *cb_data)
> >  {
> > +	const char *name_nons = strip_namespace(name);
> >  	struct strbuf *buf = cb_data;
> >  	struct object *o = parse_object(sha1);
> >  	if (!o)
> >  		return 0;
> >  
> > -	strbuf_addf(buf, "%s\t%s\n", sha1_to_hex(sha1), name);
> > +	strbuf_addf(buf, "%s\t%s\n", sha1_to_hex(sha1), name_nons);
> >  	if (o->type == OBJ_TAG) {
> >  		o = deref_tag(o, name, 0);
> >  		if (!o)
> >  			return 0;
> > -		strbuf_addf(buf, "%s\t%s^{}\n", sha1_to_hex(o->sha1), name);
> > +		strbuf_addf(buf, "%s\t%s^{}\n", sha1_to_hex(o->sha1),
> > +		            name_nons);
> >  	}
> >  	return 0;
> >  }
> > @@ -402,7 +404,7 @@ static void get_info_refs(char *arg)
> >  
> >  	} else {
> >  		select_getanyfile();
> > -		for_each_ref(show_text_ref, &buf);
> > +		for_each_namespaced_ref(show_text_ref, &buf);
> >  		send_strbuf("text/plain", &buf);
> >  	}
> >  	strbuf_release(&buf);
> > diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
> > index 938b4cf..ad85537 100644
> > --- a/t/lib-httpd/apache.conf
> > +++ b/t/lib-httpd/apache.conf
> > @@ -61,6 +61,11 @@ Alias /auth/dumb/ www/auth/dumb/
> >  	SetEnv GIT_COMMITTER_NAME "Custom User"
> >  	SetEnv GIT_COMMITTER_EMAIL custom@example.com
> >  </LocationMatch>
> > +<LocationMatch /smart_namespace/>
> > +	SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}
> > +	SetEnv GIT_HTTP_EXPORT_ALL
> > +	SetEnv GIT_NAMESPACE ns
> > +</LocationMatch>
> >  ScriptAliasMatch /smart_*[^/]*/(.*) ${GIT_EXEC_PATH}/git-http-backend/$1
> >  ScriptAlias /broken_smart/ broken-smart-http.sh/
> >  <Directory ${GIT_EXEC_PATH}>
> > diff --git a/t/t5561-http-backend.sh b/t/t5561-http-backend.sh
> > index b5d7fbc..5a19d61 100755
> > --- a/t/t5561-http-backend.sh
> > +++ b/t/t5561-http-backend.sh
> > @@ -134,6 +134,10 @@ POST /smart/repo.git/git-receive-pack HTTP/1.1 200 -
> >  ###
> >  GET  /smart/repo.git/info/refs?service=git-receive-pack HTTP/1.1 403 -
> >  POST /smart/repo.git/git-receive-pack HTTP/1.1 403 -
> > +
> > +###  namespace test
> > +###
> > +GET  /smart_namespace/repo.git/info/refs HTTP/1.1 200
> >  EOF
> >  test_expect_success 'server request log matches test results' '
> >  	sed -e "
> > diff --git a/t/t556x_common b/t/t556x_common
> > index 82926cf..cb9eb9d 100755
> > --- a/t/t556x_common
> > +++ b/t/t556x_common
> > @@ -120,3 +120,19 @@ test_expect_success 'http.receivepack false' '
> >  	GET info/refs?service=git-receive-pack "403 Forbidden" &&
> >  	POST git-receive-pack 0000 "403 Forbidden"
> >  '
> > +test_expect_success 'backend respects namespaces' '
> > +	log_div "namespace test"
> > +	config http.uploadpack true &&
> > +	config http.getanyfile true &&
> > +
> > +	GIT_NAMESPACE=ns && export GIT_NAMESPACE &&
> > +	git push public master:master &&
> > +	(cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
> > +		git for-each-ref | grep /$GIT_NAMESPACE/ >/dev/null
> > +	) &&
> > +
> > +	git ls-remote public >exp &&  
> > +	curl "$HTTPD_URL/smart_namespace/repo.git/info/refs" >act &&
> > +	test_cmp exp act &&
> > +	(grep /ns/ exp && false || true)
> > +'