* Does smart-http need git-daemon-export-ok?
@ 2009-12-26 16:21 Tarmigan
2009-12-26 17:33 ` Junio C Hamano
0 siblings, 1 reply; 14+ messages in thread
From: Tarmigan @ 2009-12-26 16:21 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Git Mailing List, Tay Ray Chuan, Clemens Buchacher, J.H.
Hi all,
Should the git-http-backend check something like git-daemon-export-ok
before serving a repository? It would have been better to discuss
this a month or two ago, but it's probably not too late since
smart-http is still so new in released versions.
I think it's very similar to git-daemon which requires that to be set,
and I think the same arguments could be made for the same kind of
check. There are already parallels for the upload-pack and
receive-pack services between the two.
Just as git-daemon may be invoked with --export-all, for
git-http-backend we could have an environmental variable to export all
repositories.
Thoughts?
Thanks,
Tarmigan
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Does smart-http need git-daemon-export-ok?
2009-12-26 16:21 Does smart-http need git-daemon-export-ok? Tarmigan
@ 2009-12-26 17:33 ` Junio C Hamano
2009-12-26 23:29 ` [PATCH 1/2] Smart-http: Add tests and documentation for export-ok Tarmigan Casebolt
2009-12-27 21:06 ` Does smart-http need git-daemon-export-ok? Shawn O. Pearce
0 siblings, 2 replies; 14+ messages in thread
From: Junio C Hamano @ 2009-12-26 17:33 UTC (permalink / raw)
To: Tarmigan
Cc: Shawn O. Pearce, Git Mailing List, Tay Ray Chuan,
Clemens Buchacher, J.H.
Tarmigan <tarmigan+git@gmail.com> writes:
> Should the git-http-backend check something like git-daemon-export-ok
> before serving a repository?
I'd agree that it would make sense to have a way to mark individual
repository for (or not for) export.
In "native" case, the chain of events are: client talks to the daemon, the
daemon checks and decides to (or not to) export, and it runs upload-pack.
In "smart http" case, http-backend is one half of what corresponds to the
daemon (the other half being your http server configuration), and it is
more flexible and git specific half, so I'd say it would make sense to
implement the check that honors the same git-daemon-export-ok flag file in
it.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] Smart-http: Add tests and documentation for export-ok
2009-12-26 17:33 ` Junio C Hamano
@ 2009-12-26 23:29 ` Tarmigan Casebolt
2009-12-26 23:29 ` [PATCH 2/2] Smart-http: check if repository is OK to export before serving it Tarmigan Casebolt
2009-12-27 21:06 ` Does smart-http need git-daemon-export-ok? Shawn O. Pearce
1 sibling, 1 reply; 14+ messages in thread
From: Tarmigan Casebolt @ 2009-12-26 23:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: spearce, git, rctay89, drizzd, warthog9, Tarmigan Casebolt
Add some tests for having smart-http check whether a repository is ok
to export. Add tests for the GIT_HTTP_EXPORT_ALL environmental
variable and checking the git-daemon-export-ok file, while leaving
existing tests still functional.
Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
---
Documentation/git-http-backend.txt | 10 +++++++++
t/lib-httpd/apache.conf | 5 ++++
t/t5560-http-backend.sh | 39 ++++++++++++++++++++++++++++++++++-
3 files changed, 52 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-http-backend.txt b/Documentation/git-http-backend.txt
index 67aec06..c8fe08a 100644
--- a/Documentation/git-http-backend.txt
+++ b/Documentation/git-http-backend.txt
@@ -18,6 +18,11 @@ The program supports clients fetching using both the smart HTTP protcol
and the backwards-compatible dumb HTTP protocol, as well as clients
pushing using the smart HTTP protocol.
+It verifies that the directory has the magic file
+"git-daemon-export-ok", and it will refuse to export any git directory
+that hasn't explicitly been marked for export this way (unless the
+GIT_HTTP_EXPORT_ALL environmental variable is set).
+
By default, only the `upload-pack` service is enabled, which serves
'git-fetch-pack' and 'git-ls-remote' clients, which are invoked from
'git-fetch', 'git-pull', and 'git-clone'. If the client is authenticated,
@@ -70,6 +75,7 @@ Apache 2.x::
+
----------------------------------------------------------------
SetEnv GIT_PROJECT_ROOT /var/www/git
+SetEnv GIT_HTTP_EXPORT_ALL
ScriptAlias /git/ /usr/libexec/git-core/git-http-backend/
----------------------------------------------------------------
+
@@ -157,6 +163,10 @@ by the invoking web server, including:
* QUERY_STRING
* REQUEST_METHOD
+The GIT_HTTP_EXPORT_ALL environmental variable may be passed to
+'git-http-backend' to bypass the check for the "git-daemon-export-ok"
+file in each repository before allowing export of that repository.
+
The backend process sets GIT_COMMITTER_NAME to '$REMOTE_USER' and
GIT_COMMITTER_EMAIL to '$\{REMOTE_USER}@http.$\{REMOTE_ADDR\}',
ensuring that any reflogs created by 'git-receive-pack' contain some
diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index 0fe3fd0..4961505 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -22,8 +22,13 @@ Alias /dumb/ www/
<Location /smart/>
SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}
+ SetEnv GIT_HTTP_EXPORT_ALL
+</Location>
+<Location /smart_noexport/>
+ SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}
</Location>
ScriptAlias /smart/ ${GIT_EXEC_PATH}/git-http-backend/
+ScriptAlias /smart_noexport/ ${GIT_EXEC_PATH}/git-http-backend/
<Directory ${GIT_EXEC_PATH}>
Options None
</Directory>
diff --git a/t/t5560-http-backend.sh b/t/t5560-http-backend.sh
index ed034bc..f763880 100755
--- a/t/t5560-http-backend.sh
+++ b/t/t5560-http-backend.sh
@@ -23,7 +23,7 @@ config() {
}
GET() {
- curl --include "$HTTPD_URL/smart/repo.git/$1" >out 2>/dev/null &&
+ curl --include "$HTTPD_URL/$SMART/repo.git/$1" >out 2>/dev/null &&
tr '\015' Q <out |
sed '
s/Q$//
@@ -91,6 +91,20 @@ get_static_files() {
GET $IDX_URL "$1"
}
+SMART=smart_noexport
+test_expect_success 'no export by default' '
+ log_div "no git-daemon-export-ok"
+ get_static_files "404 Not Found"
+'
+test_expect_success 'export if git-daemon-export-ok' '
+ log_div "git-daemon-export-ok"
+ (cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+ touch git-daemon-export-ok
+ ) &&
+ get_static_files "200 OK"
+'
+
+SMART=smart
test_expect_success 'direct refs/heads/master not found' '
log_div "refs/heads/master"
GET refs/heads/master "404 Not Found"
@@ -145,7 +159,6 @@ test_expect_success 'http.receivepack false' '
GET info/refs?service=git-receive-pack "403 Forbidden" &&
POST git-receive-pack 0000 "403 Forbidden"
'
-
run_backend() {
REQUEST_METHOD=GET \
GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
@@ -179,6 +192,28 @@ test_expect_success 'http-backend blocks bad PATH_INFO' '
cat >exp <<EOF
+### no git-daemon-export-ok
+###
+GET /smart_noexport/repo.git/HEAD HTTP/1.1 404 -
+GET /smart_noexport/repo.git/info/refs HTTP/1.1 404 -
+GET /smart_noexport/repo.git/objects/info/packs HTTP/1.1 404 -
+GET /smart_noexport/repo.git/objects/info/alternates HTTP/1.1 404 -
+GET /smart_noexport/repo.git/objects/info/http-alternates HTTP/1.1 404 -
+GET /smart_noexport/repo.git/$LOOSE_URL HTTP/1.1 404 -
+GET /smart_noexport/repo.git/$PACK_URL HTTP/1.1 404 -
+GET /smart_noexport/repo.git/$IDX_URL HTTP/1.1 404 -
+
+### git-daemon-export-ok
+###
+GET /smart_noexport/repo.git/HEAD HTTP/1.1 200
+GET /smart_noexport/repo.git/info/refs HTTP/1.1 200
+GET /smart_noexport/repo.git/objects/info/packs HTTP/1.1 200
+GET /smart_noexport/repo.git/objects/info/alternates HTTP/1.1 200 -
+GET /smart_noexport/repo.git/objects/info/http-alternates HTTP/1.1 200 -
+GET /smart_noexport/repo.git/$LOOSE_URL HTTP/1.1 200
+GET /smart_noexport/repo.git/$PACK_URL HTTP/1.1 200
+GET /smart_noexport/repo.git/$IDX_URL HTTP/1.1 200
+
### refs/heads/master
###
GET /smart/repo.git/refs/heads/master HTTP/1.1 404 -
--
1.6.6.2.g5daf2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/2] Smart-http: check if repository is OK to export before serving it
2009-12-26 23:29 ` [PATCH 1/2] Smart-http: Add tests and documentation for export-ok Tarmigan Casebolt
@ 2009-12-26 23:29 ` Tarmigan Casebolt
2009-12-27 21:10 ` Shawn O. Pearce
0 siblings, 1 reply; 14+ messages in thread
From: Tarmigan Casebolt @ 2009-12-26 23:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: spearce, git, rctay89, drizzd, warthog9, Tarmigan Casebolt
Similar to how git-daemon checks whether a repository is OK to be
exported, smart-http should also check. This check can be satisfied
in two different ways: the environmental variable GIT_HTTP_EXPORT_ALL
may be set to export all repositories, or the individual repository
may have the file git-daemon-export-ok.
Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
---
http-backend.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/http-backend.c b/http-backend.c
index f729488..345c12b 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -648,6 +648,9 @@ int main(int argc, char **argv)
setup_path();
if (!enter_repo(dir, 0))
not_found("Not a git repository: '%s'", dir);
+ if (!getenv("GIT_HTTP_EXPORT_ALL") &&
+ access("git-daemon-export-ok", F_OK) )
+ not_found("Repository not exported: '%s'", dir);
git_config(http_config, NULL);
cmd->imp(cmd_arg);
--
1.6.6.2.g5daf2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: Does smart-http need git-daemon-export-ok?
2009-12-26 17:33 ` Junio C Hamano
2009-12-26 23:29 ` [PATCH 1/2] Smart-http: Add tests and documentation for export-ok Tarmigan Casebolt
@ 2009-12-27 21:06 ` Shawn O. Pearce
1 sibling, 0 replies; 14+ messages in thread
From: Shawn O. Pearce @ 2009-12-27 21:06 UTC (permalink / raw)
To: Junio C Hamano
Cc: Tarmigan, Git Mailing List, Tay Ray Chuan, Clemens Buchacher,
J.H.
Junio C Hamano <gitster@pobox.com> wrote:
> Tarmigan <tarmigan+git@gmail.com> writes:
> > Should the git-http-backend check something like git-daemon-export-ok
> > before serving a repository?
>
> I'd agree that it would make sense to have a way to mark individual
> repository for (or not for) export.
Just for some background... early drafts of git-http-backend actually
did check for, and require, this file before it exported a repository.
I took the check out because I was relying on the HTTP server's
document root translation to provide the mapping into the local
filesystem. That meant the HTTP repository was already exported via
dumb-http, and the git-daemon-export-ok flag wasn't being checked.
Later in the series development we got the patch to allow a
different filesystem root via an environment variable, which means
its possible to hide repositories and make them available only
through git-http-backend. In that configuration, checking the
git-daemon-export-ok flag makes sense again.
> In "native" case, the chain of events are: client talks to the daemon, the
> daemon checks and decides to (or not to) export, and it runs upload-pack.
>
> In "smart http" case, http-backend is one half of what corresponds to the
> daemon (the other half being your http server configuration), and it is
> more flexible and git specific half, so I'd say it would make sense to
> implement the check that honors the same git-daemon-export-ok flag file in
> it.
Yea, I'd agree.
--
Shawn.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] Smart-http: check if repository is OK to export before serving it
2009-12-26 23:29 ` [PATCH 2/2] Smart-http: check if repository is OK to export before serving it Tarmigan Casebolt
@ 2009-12-27 21:10 ` Shawn O. Pearce
2009-12-28 4:07 ` Tarmigan
0 siblings, 1 reply; 14+ messages in thread
From: Shawn O. Pearce @ 2009-12-27 21:10 UTC (permalink / raw)
To: Tarmigan Casebolt; +Cc: Junio C Hamano, git, rctay89, drizzd, warthog9
Tarmigan Casebolt <tarmigan+git@gmail.com> wrote:
> Similar to how git-daemon checks whether a repository is OK to be
> exported, smart-http should also check. This check can be satisfied
> in two different ways: the environmental variable GIT_HTTP_EXPORT_ALL
> may be set to export all repositories, or the individual repository
> may have the file git-daemon-export-ok.
>
> Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
Acked-by: Shawn O. Pearce <spearce@spearce.org>
I really think this and 1/2 should be squashed together, in which
case you can apply my ACK to the entire thing.
--
Shawn.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] Smart-http: check if repository is OK to export before serving it
2009-12-27 21:10 ` Shawn O. Pearce
@ 2009-12-28 4:07 ` Tarmigan
2009-12-28 4:22 ` [PATCH] " Tarmigan Casebolt
2009-12-28 15:59 ` [PATCH 2/2] " Shawn O. Pearce
0 siblings, 2 replies; 14+ messages in thread
From: Tarmigan @ 2009-12-28 4:07 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, git, rctay89, drizzd, warthog9
On Sun, Dec 27, 2009 at 4:10 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
> Tarmigan Casebolt <tarmigan+git@gmail.com> wrote:
>> Similar to how git-daemon checks whether a repository is OK to be
>> exported, smart-http should also check. This check can be satisfied
>> in two different ways: the environmental variable GIT_HTTP_EXPORT_ALL
>> may be set to export all repositories, or the individual repository
>> may have the file git-daemon-export-ok.
>>
>> Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
>
> Acked-by: Shawn O. Pearce <spearce@spearce.org>
>
> I really think this and 1/2 should be squashed together, in which
> case you can apply my ACK to the entire thing.
Great, thanks for the ACK.
Squashing sounds good to me, I just split it so someone could verify
that the tests fail first if they want.
I've been thinking that the not_found() to a forbidden() instead.
Thoughts? I'll send out a unified patch with that change in a reply.
Thanks,
Tarmigan
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] Smart-http: check if repository is OK to export before serving it
2009-12-28 4:07 ` Tarmigan
@ 2009-12-28 4:22 ` Tarmigan Casebolt
2009-12-28 15:59 ` [PATCH 2/2] " Shawn O. Pearce
1 sibling, 0 replies; 14+ messages in thread
From: Tarmigan Casebolt @ 2009-12-28 4:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Tarmigan Casebolt
Similar to how git-daemon checks whether a repository is OK to be
exported, smart-http should also check. This check can be satisfied
in two different ways: the environmental variable GIT_HTTP_EXPORT_ALL
may be set to export all repositories, or the individual repository
may have the file git-daemon-export-ok.
Acked-by: Shawn O. Pearce <spearce@spearce.org>
Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
---
Documentation/git-http-backend.txt | 10 +++++++++
http-backend.c | 3 ++
t/lib-httpd/apache.conf | 5 ++++
t/t5560-http-backend.sh | 39 ++++++++++++++++++++++++++++++++++-
4 files changed, 55 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-http-backend.txt b/Documentation/git-http-backend.txt
index 67aec06..c8fe08a 100644
--- a/Documentation/git-http-backend.txt
+++ b/Documentation/git-http-backend.txt
@@ -18,6 +18,11 @@ The program supports clients fetching using both the smart HTTP protcol
and the backwards-compatible dumb HTTP protocol, as well as clients
pushing using the smart HTTP protocol.
+It verifies that the directory has the magic file
+"git-daemon-export-ok", and it will refuse to export any git directory
+that hasn't explicitly been marked for export this way (unless the
+GIT_HTTP_EXPORT_ALL environmental variable is set).
+
By default, only the `upload-pack` service is enabled, which serves
'git-fetch-pack' and 'git-ls-remote' clients, which are invoked from
'git-fetch', 'git-pull', and 'git-clone'. If the client is authenticated,
@@ -70,6 +75,7 @@ Apache 2.x::
+
----------------------------------------------------------------
SetEnv GIT_PROJECT_ROOT /var/www/git
+SetEnv GIT_HTTP_EXPORT_ALL
ScriptAlias /git/ /usr/libexec/git-core/git-http-backend/
----------------------------------------------------------------
+
@@ -157,6 +163,10 @@ by the invoking web server, including:
* QUERY_STRING
* REQUEST_METHOD
+The GIT_HTTP_EXPORT_ALL environmental variable may be passed to
+'git-http-backend' to bypass the check for the "git-daemon-export-ok"
+file in each repository before allowing export of that repository.
+
The backend process sets GIT_COMMITTER_NAME to '$REMOTE_USER' and
GIT_COMMITTER_EMAIL to '$\{REMOTE_USER}@http.$\{REMOTE_ADDR\}',
ensuring that any reflogs created by 'git-receive-pack' contain some
diff --git a/http-backend.c b/http-backend.c
index f729488..9de85cb 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -648,6 +648,9 @@ int main(int argc, char **argv)
setup_path();
if (!enter_repo(dir, 0))
not_found("Not a git repository: '%s'", dir);
+ if (!getenv("GIT_HTTP_EXPORT_ALL") &&
+ access("git-daemon-export-ok", F_OK) )
+ forbidden("Repository not exported: '%s'", dir);
git_config(http_config, NULL);
cmd->imp(cmd_arg);
diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index 0fe3fd0..4961505 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -22,8 +22,13 @@ Alias /dumb/ www/
<Location /smart/>
SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}
+ SetEnv GIT_HTTP_EXPORT_ALL
+</Location>
+<Location /smart_noexport/>
+ SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}
</Location>
ScriptAlias /smart/ ${GIT_EXEC_PATH}/git-http-backend/
+ScriptAlias /smart_noexport/ ${GIT_EXEC_PATH}/git-http-backend/
<Directory ${GIT_EXEC_PATH}>
Options None
</Directory>
diff --git a/t/t5560-http-backend.sh b/t/t5560-http-backend.sh
index ed034bc..126f6d5 100755
--- a/t/t5560-http-backend.sh
+++ b/t/t5560-http-backend.sh
@@ -23,7 +23,7 @@ config() {
}
GET() {
- curl --include "$HTTPD_URL/smart/repo.git/$1" >out 2>/dev/null &&
+ curl --include "$HTTPD_URL/$SMART/repo.git/$1" >out 2>/dev/null &&
tr '\015' Q <out |
sed '
s/Q$//
@@ -91,6 +91,20 @@ get_static_files() {
GET $IDX_URL "$1"
}
+SMART=smart_noexport
+test_expect_success 'no export by default' '
+ log_div "no git-daemon-export-ok"
+ get_static_files "403 Forbidden"
+'
+test_expect_success 'export if git-daemon-export-ok' '
+ log_div "git-daemon-export-ok"
+ (cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+ touch git-daemon-export-ok
+ ) &&
+ get_static_files "200 OK"
+'
+
+SMART=smart
test_expect_success 'direct refs/heads/master not found' '
log_div "refs/heads/master"
GET refs/heads/master "404 Not Found"
@@ -145,7 +159,6 @@ test_expect_success 'http.receivepack false' '
GET info/refs?service=git-receive-pack "403 Forbidden" &&
POST git-receive-pack 0000 "403 Forbidden"
'
-
run_backend() {
REQUEST_METHOD=GET \
GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
@@ -179,6 +192,28 @@ test_expect_success 'http-backend blocks bad PATH_INFO' '
cat >exp <<EOF
+### no git-daemon-export-ok
+###
+GET /smart_noexport/repo.git/HEAD HTTP/1.1 403 -
+GET /smart_noexport/repo.git/info/refs HTTP/1.1 403 -
+GET /smart_noexport/repo.git/objects/info/packs HTTP/1.1 403 -
+GET /smart_noexport/repo.git/objects/info/alternates HTTP/1.1 403 -
+GET /smart_noexport/repo.git/objects/info/http-alternates HTTP/1.1 403 -
+GET /smart_noexport/repo.git/$LOOSE_URL HTTP/1.1 403 -
+GET /smart_noexport/repo.git/$PACK_URL HTTP/1.1 403 -
+GET /smart_noexport/repo.git/$IDX_URL HTTP/1.1 403 -
+
+### git-daemon-export-ok
+###
+GET /smart_noexport/repo.git/HEAD HTTP/1.1 200
+GET /smart_noexport/repo.git/info/refs HTTP/1.1 200
+GET /smart_noexport/repo.git/objects/info/packs HTTP/1.1 200
+GET /smart_noexport/repo.git/objects/info/alternates HTTP/1.1 200 -
+GET /smart_noexport/repo.git/objects/info/http-alternates HTTP/1.1 200 -
+GET /smart_noexport/repo.git/$LOOSE_URL HTTP/1.1 200
+GET /smart_noexport/repo.git/$PACK_URL HTTP/1.1 200
+GET /smart_noexport/repo.git/$IDX_URL HTTP/1.1 200
+
### refs/heads/master
###
GET /smart/repo.git/refs/heads/master HTTP/1.1 404 -
--
1.6.6.1.g8eede.dirty
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] Smart-http: check if repository is OK to export before serving it
2009-12-28 4:07 ` Tarmigan
2009-12-28 4:22 ` [PATCH] " Tarmigan Casebolt
@ 2009-12-28 15:59 ` Shawn O. Pearce
2009-12-28 16:57 ` Tarmigan
1 sibling, 1 reply; 14+ messages in thread
From: Shawn O. Pearce @ 2009-12-28 15:59 UTC (permalink / raw)
To: Tarmigan; +Cc: Junio C Hamano, git, rctay89, drizzd, warthog9
Tarmigan <tarmigan+git@gmail.com> wrote:
> On Sun, Dec 27, 2009 at 4:10 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
> > Tarmigan Casebolt <tarmigan+git@gmail.com> wrote:
> >> Similar to how git-daemon checks whether a repository is OK to be
> >> exported, smart-http should also check. ?This check can be satisfied
> >> in two different ways: the environmental variable GIT_HTTP_EXPORT_ALL
> >> may be set to export all repositories, or the individual repository
> >> may have the file git-daemon-export-ok.
...
> I've been thinking that the not_found() to a forbidden() instead.
Oh. Interesting question.
Because you can't resolve the access error by authenticating to
the server, we may actually want to just return not_found() here
with a message in the log of "Repository not exported: '%s'".
That would mirror the behavior of git-daemon.
--
Shawn.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] Smart-http: check if repository is OK to export before serving it
2009-12-28 15:59 ` [PATCH 2/2] " Shawn O. Pearce
@ 2009-12-28 16:57 ` Tarmigan
2009-12-28 17:08 ` Shawn O. Pearce
0 siblings, 1 reply; 14+ messages in thread
From: Tarmigan @ 2009-12-28 16:57 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, git, rctay89, drizzd, warthog9
On Mon, Dec 28, 2009 at 10:59 AM, Shawn O. Pearce <spearce@spearce.org> wrote:
> Tarmigan <tarmigan+git@gmail.com> wrote:
>> I've been thinking that the not_found() to a forbidden() instead.
>
> Oh. Interesting question.
>
> Because you can't resolve the access error by authenticating to
> the server, we may actually want to just return not_found() here
> with a message in the log of "Repository not exported: '%s'".
I'm no http expert, but isn't that what 401 would be? From
http://tools.ietf.org/html/rfc2616#section-10.4.4
403 Forbidden
The server understood the request, but is refusing to fulfill it.
Authorization will not help and the request SHOULD NOT be repeated.
If the request method was not HEAD and the server wishes to make
public why the request has not been fulfilled, it SHOULD describe the
reason for the refusal in the entity. If the server does not wish to
make this information available to the client, the status code 404
(Not Found) can be used instead.
which to me points to 403 instead of 404.
I don't have a strong preference, and will resend with those changes
if you'd prefer 404.
Thanks,
Tarmigan
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] Smart-http: check if repository is OK to export before serving it
2009-12-28 16:57 ` Tarmigan
@ 2009-12-28 17:08 ` Shawn O. Pearce
2009-12-28 21:49 ` [PATCH] " Tarmigan Casebolt
0 siblings, 1 reply; 14+ messages in thread
From: Shawn O. Pearce @ 2009-12-28 17:08 UTC (permalink / raw)
To: Tarmigan; +Cc: Junio C Hamano, git, rctay89, drizzd, warthog9
Tarmigan <tarmigan+git@gmail.com> wrote:
> On Mon, Dec 28, 2009 at 10:59 AM, Shawn O. Pearce <spearce@spearce.org> wrote:
> > Tarmigan <tarmigan+git@gmail.com> wrote:
> >> I've been thinking that the not_found() to a forbidden() instead.
> >
> > Because you can't resolve the access error by authenticating to
> > the server, we may actually want to just return not_found() here
> > with a message in the log of "Repository not exported: '%s'".
>
> I'm no http expert, but isn't that what 401 would be? From
> http://tools.ietf.org/html/rfc2616#section-10.4.4
> 403 Forbidden
> The server understood the request, but is refusing to fulfill it.
> Authorization will not help and the request SHOULD NOT be repeated.
> If the request method was not HEAD and the server wishes to make
> public why the request has not been fulfilled, it SHOULD describe the
> reason for the refusal in the entity. If the server does not wish to
> make this information available to the client, the status code 404
> (Not Found) can be used instead.
> which to me points to 403 instead of 404.
Good point, that is 403. But the last sentance leads me to believe
404 might be a better use here. Under git-daemon we don't tell
the client the difference between "Not Found" and "Not Exported",
so I think we should be doing the same thing here under HTTP.
--
Shawn.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] Smart-http: check if repository is OK to export before serving it
2009-12-28 17:08 ` Shawn O. Pearce
@ 2009-12-28 21:49 ` Tarmigan Casebolt
2009-12-29 9:19 ` Junio C Hamano
0 siblings, 1 reply; 14+ messages in thread
From: Tarmigan Casebolt @ 2009-12-28 21:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, spearce, Tarmigan Casebolt
Similar to how git-daemon checks whether a repository is OK to be
exported, smart-http should also check. This check can be satisfied
in two different ways: the environmental variable GIT_HTTP_EXPORT_ALL
may be set to export all repositories, or the individual repository
may have the file git-daemon-export-ok.
Acked-by: Shawn O. Pearce <spearce@spearce.org>
Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
---
OK, I see what you're saying Shawn. I've changed it back to "404
Not Found" again.
I've also reordered the new tests since the last time I sent it out.
The new tests use the same test as in
"static file is ok"
so put the new tests after that test in case that test breaks.
Documentation/git-http-backend.txt | 10 +++++++++
http-backend.c | 3 ++
t/lib-httpd/apache.conf | 5 ++++
t/t5560-http-backend.sh | 39 ++++++++++++++++++++++++++++++++++-
4 files changed, 55 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-http-backend.txt b/Documentation/git-http-backend.txt
index 67aec06..c8fe08a 100644
--- a/Documentation/git-http-backend.txt
+++ b/Documentation/git-http-backend.txt
@@ -18,6 +18,11 @@ The program supports clients fetching using both the smart HTTP protcol
and the backwards-compatible dumb HTTP protocol, as well as clients
pushing using the smart HTTP protocol.
+It verifies that the directory has the magic file
+"git-daemon-export-ok", and it will refuse to export any git directory
+that hasn't explicitly been marked for export this way (unless the
+GIT_HTTP_EXPORT_ALL environmental variable is set).
+
By default, only the `upload-pack` service is enabled, which serves
'git-fetch-pack' and 'git-ls-remote' clients, which are invoked from
'git-fetch', 'git-pull', and 'git-clone'. If the client is authenticated,
@@ -70,6 +75,7 @@ Apache 2.x::
+
----------------------------------------------------------------
SetEnv GIT_PROJECT_ROOT /var/www/git
+SetEnv GIT_HTTP_EXPORT_ALL
ScriptAlias /git/ /usr/libexec/git-core/git-http-backend/
----------------------------------------------------------------
+
@@ -157,6 +163,10 @@ by the invoking web server, including:
* QUERY_STRING
* REQUEST_METHOD
+The GIT_HTTP_EXPORT_ALL environmental variable may be passed to
+'git-http-backend' to bypass the check for the "git-daemon-export-ok"
+file in each repository before allowing export of that repository.
+
The backend process sets GIT_COMMITTER_NAME to '$REMOTE_USER' and
GIT_COMMITTER_EMAIL to '$\{REMOTE_USER}@http.$\{REMOTE_ADDR\}',
ensuring that any reflogs created by 'git-receive-pack' contain some
diff --git a/http-backend.c b/http-backend.c
index f729488..345c12b 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -648,6 +648,9 @@ int main(int argc, char **argv)
setup_path();
if (!enter_repo(dir, 0))
not_found("Not a git repository: '%s'", dir);
+ if (!getenv("GIT_HTTP_EXPORT_ALL") &&
+ access("git-daemon-export-ok", F_OK) )
+ not_found("Repository not exported: '%s'", dir);
git_config(http_config, NULL);
cmd->imp(cmd_arg);
diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index 0fe3fd0..4961505 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -22,8 +22,13 @@ Alias /dumb/ www/
<Location /smart/>
SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}
+ SetEnv GIT_HTTP_EXPORT_ALL
+</Location>
+<Location /smart_noexport/>
+ SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}
</Location>
ScriptAlias /smart/ ${GIT_EXEC_PATH}/git-http-backend/
+ScriptAlias /smart_noexport/ ${GIT_EXEC_PATH}/git-http-backend/
<Directory ${GIT_EXEC_PATH}>
Options None
</Directory>
diff --git a/t/t5560-http-backend.sh b/t/t5560-http-backend.sh
index ed034bc..04a9896 100755
--- a/t/t5560-http-backend.sh
+++ b/t/t5560-http-backend.sh
@@ -23,7 +23,7 @@ config() {
}
GET() {
- curl --include "$HTTPD_URL/smart/repo.git/$1" >out 2>/dev/null &&
+ curl --include "$HTTPD_URL/$SMART/repo.git/$1" >out 2>/dev/null &&
tr '\015' Q <out |
sed '
s/Q$//
@@ -91,6 +91,7 @@ get_static_files() {
GET $IDX_URL "$1"
}
+SMART=smart
test_expect_success 'direct refs/heads/master not found' '
log_div "refs/heads/master"
GET refs/heads/master "404 Not Found"
@@ -99,6 +100,19 @@ test_expect_success 'static file is ok' '
log_div "getanyfile default"
get_static_files "200 OK"
'
+SMART=smart_noexport
+test_expect_success 'no export by default' '
+ log_div "no git-daemon-export-ok"
+ get_static_files "404 Not Found"
+'
+test_expect_success 'export if git-daemon-export-ok' '
+ log_div "git-daemon-export-ok"
+ (cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+ touch git-daemon-export-ok
+ ) &&
+ get_static_files "200 OK"
+'
+SMART=smart
test_expect_success 'static file if http.getanyfile true is ok' '
log_div "getanyfile true"
config http.getanyfile true &&
@@ -145,7 +159,6 @@ test_expect_success 'http.receivepack false' '
GET info/refs?service=git-receive-pack "403 Forbidden" &&
POST git-receive-pack 0000 "403 Forbidden"
'
-
run_backend() {
REQUEST_METHOD=GET \
GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
@@ -194,6 +207,28 @@ GET /smart/repo.git/$LOOSE_URL HTTP/1.1 200
GET /smart/repo.git/$PACK_URL HTTP/1.1 200
GET /smart/repo.git/$IDX_URL HTTP/1.1 200
+### no git-daemon-export-ok
+###
+GET /smart_noexport/repo.git/HEAD HTTP/1.1 404 -
+GET /smart_noexport/repo.git/info/refs HTTP/1.1 404 -
+GET /smart_noexport/repo.git/objects/info/packs HTTP/1.1 404 -
+GET /smart_noexport/repo.git/objects/info/alternates HTTP/1.1 404 -
+GET /smart_noexport/repo.git/objects/info/http-alternates HTTP/1.1 404 -
+GET /smart_noexport/repo.git/$LOOSE_URL HTTP/1.1 404 -
+GET /smart_noexport/repo.git/$PACK_URL HTTP/1.1 404 -
+GET /smart_noexport/repo.git/$IDX_URL HTTP/1.1 404 -
+
+### git-daemon-export-ok
+###
+GET /smart_noexport/repo.git/HEAD HTTP/1.1 200
+GET /smart_noexport/repo.git/info/refs HTTP/1.1 200
+GET /smart_noexport/repo.git/objects/info/packs HTTP/1.1 200
+GET /smart_noexport/repo.git/objects/info/alternates HTTP/1.1 200 -
+GET /smart_noexport/repo.git/objects/info/http-alternates HTTP/1.1 200 -
+GET /smart_noexport/repo.git/$LOOSE_URL HTTP/1.1 200
+GET /smart_noexport/repo.git/$PACK_URL HTTP/1.1 200
+GET /smart_noexport/repo.git/$IDX_URL HTTP/1.1 200
+
### getanyfile true
###
GET /smart/repo.git/HEAD HTTP/1.1 200
--
1.6.6.1.g8d7b9
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] Smart-http: check if repository is OK to export before serving it
2009-12-28 21:49 ` [PATCH] " Tarmigan Casebolt
@ 2009-12-29 9:19 ` Junio C Hamano
2009-12-29 15:00 ` Shawn O. Pearce
0 siblings, 1 reply; 14+ messages in thread
From: Junio C Hamano @ 2009-12-29 9:19 UTC (permalink / raw)
To: Tarmigan Casebolt; +Cc: git, spearce
Tarmigan Casebolt <tarmigan+git@gmail.com> writes:
> Similar to how git-daemon checks whether a repository is OK to be
> exported, smart-http should also check. This check can be satisfied
> in two different ways: the environmental variable GIT_HTTP_EXPORT_ALL
> may be set to export all repositories, or the individual repository
> may have the file git-daemon-export-ok.
>
> Acked-by: Shawn O. Pearce <spearce@spearce.org>
> Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
> ---
> OK, I see what you're saying Shawn. I've changed it back to "404
> Not Found" again.
>
> I've also reordered the new tests since the last time I sent it out.
> The new tests use the same test as in
> "static file is ok"
> so put the new tests after that test in case that test breaks.
Looks sane to me, although I am afraid that I am not as familiar with the
codepath involved as I should be. Shawn, is your Ack still good?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Smart-http: check if repository is OK to export before serving it
2009-12-29 9:19 ` Junio C Hamano
@ 2009-12-29 15:00 ` Shawn O. Pearce
0 siblings, 0 replies; 14+ messages in thread
From: Shawn O. Pearce @ 2009-12-29 15:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Tarmigan Casebolt, git
Junio C Hamano <gitster@pobox.com> wrote:
> Tarmigan Casebolt <tarmigan+git@gmail.com> writes:
>
> > Similar to how git-daemon checks whether a repository is OK to be
> > exported, smart-http should also check. This check can be satisfied
> > in two different ways: the environmental variable GIT_HTTP_EXPORT_ALL
> > may be set to export all repositories, or the individual repository
> > may have the file git-daemon-export-ok.
> >
> > Acked-by: Shawn O. Pearce <spearce@spearce.org>
...
> Looks sane to me, although I am afraid that I am not as familiar with the
> codepath involved as I should be. Shawn, is your Ack still good?
Yes, my ACK is still good. :-)
--
Shawn.
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2009-12-29 15:00 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-12-26 16:21 Does smart-http need git-daemon-export-ok? Tarmigan
2009-12-26 17:33 ` Junio C Hamano
2009-12-26 23:29 ` [PATCH 1/2] Smart-http: Add tests and documentation for export-ok Tarmigan Casebolt
2009-12-26 23:29 ` [PATCH 2/2] Smart-http: check if repository is OK to export before serving it Tarmigan Casebolt
2009-12-27 21:10 ` Shawn O. Pearce
2009-12-28 4:07 ` Tarmigan
2009-12-28 4:22 ` [PATCH] " Tarmigan Casebolt
2009-12-28 15:59 ` [PATCH 2/2] " Shawn O. Pearce
2009-12-28 16:57 ` Tarmigan
2009-12-28 17:08 ` Shawn O. Pearce
2009-12-28 21:49 ` [PATCH] " Tarmigan Casebolt
2009-12-29 9:19 ` Junio C Hamano
2009-12-29 15:00 ` Shawn O. Pearce
2009-12-27 21:06 ` Does smart-http need git-daemon-export-ok? Shawn O. Pearce
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).