Pierre Habouzit <madcoder@xxxxxxxxxx> writes:
> +static struct {
> + const char *path;
> + int use_as_fmt;
> +} base_path;
>
> /* If defined, ~user notation is allowed and the string is inserted
> * after ~user/. E.g. a request to git://host/~alice/frotz would
> * go to /home/alice/pub_git/frotz with --user-path=pub_git.
> */
> -static const char *user_path;
> +static struct {
> + const char *path;
> + int use_as_fmt;
> +} user_path;
Maybe it does not matter much, but I wonder if we want to keep
two structs the same type, like:
static struct {
const char *path;
int use_as_fmt;
} base_path, user_path;
I also wondered if we can just extend the semantics of base_path
and user_path to autodetect the fmt-ness of them, but that means
we would break existing setups that uses per-cent in the
pathname. Arguably that would not be so common and we may not
need to worry about such an installation, though. What do you
think?
> @@ -174,24 +285,45 @@ static char *path_ok(char *dir)
> slash = dir + restlen;
> namlen = slash - dir;
> restlen -= namlen;
> +
> + if (user_path.use_as_fmt) {
> + loginfo("host <%s>, "
> + "userpathfmt <%s>, request <%s>, "
> + "namlen %d, restlen %d, slash <%s>",
> + vhost,
> + user_path.path, dir,
> + namlen, restlen, slash);
> + dir = git_path_fmt(rpath, user_path.path, vhost,
> + slash, dir + 1, namlen - 1);
When vhost is NULL you would feed it to "%s", which I think
glibc works around with (null) fine but other C libraries would
not like it. git_path_fmt()'s logging does not have this
problem, though.
> + else if (base_path.path) {
> if (*dir != '/') {
> /* Allow only absolute */
> logerror("'%s': Non-absolute path denied (base-path active)", dir);
> return NULL;
> }
> +
> + if (base_path.use_as_fmt) {
> + dir = git_base_path_fmt(rpath, base_path.path, vhost, dir);
> + } else {
> + snprintf(rpath, PATH_MAX, "%s%s", base_path.path, dir);
The level of logging in this branch and in user_path.use_as_fmt
branch are inconsistent. Maybe the more detailed one above I
commented about vhost==NULL case was primarily meant for
debugging and you forgot to remove it?
> @@ -274,6 +406,7 @@ static int execute(struct sockaddr *addr
> @@ -303,15 +436,30 @@ #endif
> alarm(0);
>
> len = strlen(line);
> +
> + if (pktlen != len) {
> + int arg_pos = len + 1;
> +
> loginfo("Extended attributes (%d bytes) exist <%.*s>",
> (int) pktlen - len,
> + (int) pktlen - len, line + arg_pos);
> +
> + while (arg_pos < pktlen) {
> + int arg_len = strlen(line + arg_pos);
> +
> + if (!strncmp("host=", line + arg_pos, 5)) {
> + vhost = line + arg_pos + 5;
> + }
> +
> + arg_pos += arg_len + 1;
> + }
> + }
> +
I think it is easier to do:
if (!vhost)
vhost = default_host;
and have git_base_path_fmt() barf if the format calls for %h and
vhost passed to it is NULL. Lack of "host=" in the request is
logged here already.
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html