5:13 p.m.
repo: https://bitbucket.org/richardipsum/gitano
ref: richardipsum/fix-empty-cmd-bug
head: 1420abf7d99359a056944f2023cbe6183924a5a8
Hi,
Apologies for not submitting this sooner, limited internet access over the
past year has made it more difficult for me to contribute,
I must also apologise for introducing this bug in the first place.
Richard Ipsum (1):
Fix empty command bug
bin/gitano-auth.in | 7 ++++++-
bin/gitano-command.cgi.in | 8 +++++++-
bin/gitano-smart-http.cgi.in | 10 +++++++++-
lib/gitano/auth.lua | 8 +++-----
4 files changed, 25 insertions(+), 8 deletions(-)
--
2.1.4
5:13 p.m.
This fixes a bug that causes gitano to crash when it's given an empty
command list
Opening and closing of the log should be done in the application,
so it can log before checking for authorization, log after, and
clean up at the end
---
bin/gitano-auth.in | 7 ++++++-
bin/gitano-command.cgi.in | 8 +++++++-
bin/gitano-smart-http.cgi.in | 10 +++++++++-
lib/gitano/auth.lua | 8 +++-----
4 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/bin/gitano-auth.in b/bin/gitano-auth.in
index 2190ae0..2ec6c45 100644
--- a/bin/gitano-auth.in
+++ b/bin/gitano-auth.in
@@ -20,6 +20,10 @@ local sp = require "luxio.subprocess"
-- @@GITANO_SHARE_PATH
-- @@GITANO_PLUGIN_PATH
+local start_log_level = gitano.log.get_level()
+gitano.log.cap_level(gitano.log.level.INFO)
+local transactionid = gitano.log.syslog.open()
+
local repo_root, user, keytag = ...
gitano.config.repo_path(repo_root)
@@ -31,7 +35,8 @@ if cmdline:match("^[ \t\n]*$") then
end
local authorized, cmd, parsed_cmdline, config, env, repo =
- gitano.auth.is_authorized(user, "ssh", cmdline, repo_root)
+ gitano.auth.is_authorized(user, "ssh", cmdline, repo_root,
+ transactionid, start_log_level)
if authorized then
local exit = gitano.util.run_command(cmd, cmdline, parsed_cmdline,
diff --git a/bin/gitano-command.cgi.in b/bin/gitano-command.cgi.in
index 2fa1db2..94c5dd5 100755
--- a/bin/gitano-command.cgi.in
+++ b/bin/gitano-command.cgi.in
@@ -52,9 +52,13 @@ if os.getenv("QUERY_STRING") then
local user = os.getenv("REMOTE_USER") or "gitano/anonymous"
gitano.log.buffer_output()
+ local start_log_level = gitano.log.get_level()
+ gitano.log.cap_level(gitano.log.level.INFO)
+ local transactionid = gitano.log.syslog.open()
local authorized, cmd, parsed_cmdline, config, env, repo =
- gitano.auth.is_authorized(user, "http", cmdline,
os.getenv("GITANO_ROOT"))
+ gitano.auth.is_authorized(user, "http", cmdline,
os.getenv("GITANO_ROOT"),
+ transactionid, start_log_level)
if authorized then
local exit = gitano.util.run_command(cmd, cmdline, parsed_cmdline,
@@ -67,6 +71,8 @@ if os.getenv("QUERY_STRING") then
stdout:write("Status: 403 Forbidden\r\n\r\n")
stdout:write(gitano.log.get_buffered_output() or "")
end
+
+ gitano.log.syslog.close()
else
stdout:write("Status: 400 Bad request\r\n\r\n")
stdout:write("Malformed command line, format: ?cmd=arg0 arg1 ... argn\n")
diff --git a/bin/gitano-smart-http.cgi.in b/bin/gitano-smart-http.cgi.in
index f294b28..6f8fdee 100755
--- a/bin/gitano-smart-http.cgi.in
+++ b/bin/gitano-smart-http.cgi.in
@@ -59,8 +59,14 @@ if request_method == "GET" or request_method ==
"POST" then
local user = os.getenv("REMOTE_USER") or "gitano/anonymous"
local cmdline = parse_request(request_method)
+ local start_log_level = gitano.log.get_level()
+ gitano.log.cap_level(gitano.log.level.INFO)
+ local transactionid = gitano.log.syslog.open()
+
if cmdline and gitano.auth.is_authorized(user, "http", cmdline,
- os.getenv("GITANO_ROOT")) then
+ os.getenv("GITANO_ROOT"),
+ transactionid,
+ start_log_level) then
local proc = subprocess.spawn_simple({"git", "http-backend"})
local exit_code
@@ -72,6 +78,8 @@ if request_method == "GET" or request_method ==
"POST" then
else
stdout:write("Status: 403 Forbidden\r\n\r\n")
end
+
+ gitano.log.syslog.close()
else
stdout:write("Status: 405 Method Not Allowed\r\n")
stdout:write("Allow: GET, POST\r\n\r\n")
diff --git a/lib/gitano/auth.lua b/lib/gitano/auth.lua
index be3f9fc..2583d79 100644
--- a/lib/gitano/auth.lua
+++ b/lib/gitano/auth.lua
@@ -51,14 +51,12 @@ local function set_environment(repo_root, repo, context,
transactionid)
return env
end
-local function is_authorized(user, source, cmdline, repo_root)
+local function is_authorized(user, source, cmdline, repo_root,
+ transactionid, start_log_level)
+
local keytag = ""
local authorized = false
- local start_log_level = log.get_level()
- log.cap_level(log.level.INFO)
- local transactionid = log.syslog.open()
-
config.repo_path(repo_root)
if not user or not cmdline then
--
2.1.4
1:43 p.m.
On Fri, Jul 10, 2015 at 22:13:28 +0100, Richard Ipsum wrote:
Apologies for not submitting this sooner, limited internet access
over the
past year has made it more difficult for me to contribute,
I must also apologise for introducing this bug in the first place.
This looks sane -- have you tried running this lot under the test suite? If so
could you possibly come up with a yarn which verifies that the error messages
are useful if you don't supply a command?
D.
--
Daniel Silverstone http://www.digital-scurf.org/
PGP mail accepted and encouraged. Key Id: 3CCE BABE 206C 3B69
11:09 a.m.
On Mon, Jul 13, 2015 at 06:43:21PM +0100, Daniel Silverstone wrote:
On Fri, Jul 10, 2015 at 22:13:28 +0100, Richard Ipsum wrote:
> Apologies for not submitting this sooner, limited internet access over the
> past year has made it more difficult for me to contribute,
> I must also apologise for introducing this bug in the first place.
This looks sane -- have you tried running this lot under the test suite?
I can confirm that it doesn't break the test suite.
If so could you possibly come up with a yarn which verifies that the
error
messages are useful if you don't supply a command?
Provided the expected stderr is "[gitano] FATAL: No command provided, cannot
continue",
then these patches add that, with Richard Ipsum's patch rebased.
Repo: git://git.gitano.org.uk/personal/richardmaw/gitano.git
Ref: richardipsum/fix-empty-cmd-bug
Sha1: eddcfb9975334bd9e8b2578e45f8ab789a926887
Richard Maw (2):
yarnlib: Allow empty "WHEN foo bar runs" commands
yarns: Test behaviour when no commands given
testing/01-basics.yarn | 13 +++++++++++++
testing/library.yarn | 4 ++--
2 files changed, 15 insertions(+), 2 deletions(-)
--
1.9.1
11:09 a.m.
New subject: [PATCH 2/3] yarnlib: Allow empty "WHEN foo bar runs" commands
Rather than specialising a 0-ary version,
it's less effort to make the existing implementation flexible enough.
---
testing/library.yarn | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/testing/library.yarn b/testing/library.yarn
index 89e8da1..3f57a26 100644
--- a/testing/library.yarn
+++ b/testing/library.yarn
@@ -81,10 +81,10 @@ Admin repo manipulation
Generic utility methods
-----------------------
- IMPLEMENTS WHEN ([a-z][a-z0-9]*) ([a-z][a-z0-9]*) runs (.+)
+ IMPLEMENTS WHEN ([a-z][a-z0-9]*) ([a-z][a-z0-9]*) runs ?(.*)
$GTT runcommand $MATCH_1 $MATCH_2 $MATCH_3 > $DATADIR/stdout 2> $DATADIR/stderr
- IMPLEMENTS WHEN ([a-z][a-z0-9]*) ([a-z][a-z0-9]*),? expecting failure,? runs (.+)
+ IMPLEMENTS WHEN ([a-z][a-z0-9]*) ([a-z][a-z0-9]*),? expecting failure,? runs ?(.*)
if $GTT runcommand $MATCH_1 $MATCH_2 $MATCH_3 > $DATADIR/stdout 2>
$DATADIR/stderr; then
false
fi
--
1.9.1
11:09 a.m.
New subject: [PATCH 3/3] yarns: Test behaviour when no commands given
---
testing/01-basics.yarn | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/testing/01-basics.yarn b/testing/01-basics.yarn
index 3029f16..5667851 100644
--- a/testing/01-basics.yarn
+++ b/testing/01-basics.yarn
@@ -104,3 +104,16 @@ the `alice` user cannot see it.
THEN stdout contains RW \ anotherrepo
WHEN alice main runs ls
THEN stdout does not contain anotherrepo
+
+Basic command handling
+----------------------
+
+Users interact by running commands.
+Normally when using SSH without a command you get a shell,
+however this isn't the standard use of SSH,
+so when you haven't provided a command you get an error message.
+
+ SCENARIO handling empty commands
+ GIVEN a standard instance
+ WHEN testinstance adminkey, expecting failure, runs
+ THEN stderr contains FATAL: No command provided, cannot continue
--
1.9.1
2:05 p.m.
On Wed, Oct 28, 2015 at 15:09:46 +0000, Richard Maw wrote:
Provided the expected stderr is "[gitano] FATAL: No command
provided, cannot continue",
then these patches add that, with Richard Ipsum's patch rebased.
Wonderful, I liked Richard Ipsum's patch, so thank you for the tests.
I have merged this work, validated it, and pushed it to master.
--
Daniel Silverstone http://www.digital-scurf.org/
PGP mail accepted and encouraged. Key Id: 3CCE BABE 206C 3B69
2776
days inactive
2888
days old
6 comments
3 participants
participants (3)
-
Daniel Silverstone -
Richard Ipsum -
Richard Maw