* Re: Re: Re: Add "clone" support to lntree
From: Daniel Barkalow @ 2005-04-17 0:07 UTC (permalink / raw)
To: Petr Baudis; +Cc: git
In-Reply-To: <20050416233305.GO19099@pasky.ji.cz>
On Sun, 17 Apr 2005, Petr Baudis wrote:
> Dear diary, on Sat, Apr 16, 2005 at 05:17:00AM CEST, I got a letter
> where Daniel Barkalow <barkalow@iabervon.org> told me that...
> > On Sat, 16 Apr 2005, Petr Baudis wrote:
> >
> > > Dear diary, on Sat, Apr 16, 2005 at 04:47:55AM CEST, I got a letter
> > > where Petr Baudis <pasky@ucw.cz> told me that...
> > > > git branch --- creates a branch from a given commit
> > > > (when passed empty commit, creates a branch
> > > > from the current commit and sets the working
> > > > tree to that branch)
> > > > Note that there is a bug in current git update - it will allow you to
> > > > bring several of your trees to follow the same branch, or even a remote
> > > > branch. This is not even supposed to work, and will be fixed when I get
> > > > some sleep. You will be able to do git pull even on local branches, and
> > > > the proper solution for this will be just tracking the branch you want
> > > > to follow.
> > >
> > > I must admit that I'm not entirely decided yet, so I'd love to hear your
> > > opinion.
> > >
> > > I'm wondering, whether each tree should be fixed to a certain branch.
> > > That is, you decide a name when you do git fork, and then the tree
> > > always follows that branch. (It always has to follow [be bound to]
> > > *some* branch, and each branch can be followed by only a single tree at
> > > a time.)
> >
> > I don't think I'm following the use of branches. Currently, what I do is
> > have a git-pasky and a git-linus, and fork off a working directory from
> > one of these for each thing I want to work on. I do some work, commit as I
> > make progress, and then do a diff against the remote head to get a patch
> > to send off. If I want to do a series of patches which depend on each
> > other, I fork my next directory off of my previous one rather than off of
> > a remote base. I haven't done much rebasing, so I haven't worked out how I
> > would do that most effectively.
>
> Yes. And that's exactly what the branches allow you to do. You just do
>
> git fork myhttpclient ~/myhttpclientdir
>
> then you do some hacking, and when you have something usable, you can
> go back to your main working directory and do
>
> git merge -b when_you_started myhttpclient
>
> Since you consider the code perfect, you can now just rm -rf
> ~/myhttpclient.
>
> Suddenly, you get a mail from mj pointing out some bugs, and it looks
> like there are more to come. What to do?
>
> git fork myhttpclient ~/myhttpclientdir
>
> (Ok, this does not work, but that's a bug, will fix tomorrow.) This will
> let you take off when you left in your work on the branch.
Ah, I think that's what made me think I wasn't understanding branches; the
first thing I tried hit this big.
> git update for seeking between commits is probably extremely important
> for any kind of binary search when you are wondering when did this bug
> appeared first, or when you are exploring how certain branch evolved
> over time. Doing git fork for each successive iteration sounds horrible.
Even if there isn't a performance hit, it's semantically wrong, because
you're looking at different versions that were in the same place at
different times.
> Now, what about git branch and git update for switching between
> branches? I think this is the most controversial part; these are
> basically just shortcuts for not having to do git fork, and I wouldn't
> mind so much removing them, if you people really consider them too ugly
> a wart for the soft clean git skin. I admit that they both come from a
> hidden prejudice that git fork is going to be slow and eat a lot of
> disk.
I think that this just confuses matters.
> The idea for git update for switching between branches is that
> especially when you have two rather similar branches and mostly do stuff
> on one of them, but sometimes you want to do something on the other one,
> you can do just quick git update, do stuff, and git update back, without
> any forking.
I still think that fork should be quick enough, or you could leave the
extra tree around. I'm not against having such a command, but I think it
should be a separate command rather than a different use of update, since
it would be used by poeople working in different ways.
> > I think I can make this space efficient by hardlinking unmodified blobs to
> > a directory of cached expanded blobs.
>
> I don't know but I really feel *very* unsafe when doing that. What if
> something screws up and corrupts my base... way too easy. And it gets
> pretty inconvenient and even more dangerous when you get the idea to do
> some modifications on your tree by something else than your favorite
> editor (which you've already checked does the right thing).
It should only be an option, not required and maybe not even
default. I think it should be possible to prevent stuff from screwing up,
since we really don't want anything to ever modify those inodes (as
opposed to some cases, where you want to modify inodes only in certain
ways). For that matter, relatively few programs actually support
modifying inodes rather than unlinking.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: optimize gitdiff-do script
From: Paul Jackson @ 2005-04-17 0:10 UTC (permalink / raw)
To: Petr Baudis; +Cc: git
In-Reply-To: <20050416234344.GQ19099@pasky.ji.cz>
Petr wrote:
> Please don't reindent the scripts. It violates the current coding style
> and the patch is unreviewable.
Sorry - I had not realized that there was a style in this case.
I am all in favor of such coding styles, and will gladly fit this one.
Do you want the patch resent, or a patch to restore indent on top of
this one?
> the patch is unreviewable.
The section that I indented the wrong way was such a total rewrite, that
you aren't going to be able to review it line by line compared to the
old anyway. So in this case, it wasn't that I was modifying and
reindenting, rather that I was rewriting a page of code from scratch.
But that's a nit. Honoring the coding style is necessary in any case.
> The idea behind that was that diffing could take a significant portion
> of disk space,
Here I don't understand, or don't agree, not sure which.
This won't eat more disk space, because the same tmp files are reused,
over and over. Instead of unlinking them just before reopening them
truncating (O_WRONLY|O_CREAT|O_TRUNC), I just reopen them truncating.
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <pj@engr.sgi.com> 1.650.933.1373, 1.925.600.0401
^ permalink raw reply
* [PATCH] Use libcurl to use HTTP to get repositories
From: Daniel Barkalow @ 2005-04-17 0:14 UTC (permalink / raw)
To: git; +Cc: Linus Torvalds
This enables the use of HTTP to download commits and associated objects
from remote repositories. It now uses libcurl instead of local hack code.
Still causes warnings for fsck-cache and rev-tree, due to unshared code.
Still leaks a bit of memory due to bug copied from read-tree.
Needs libcurl post 7.7 or so.
Signed-Off-By: Daniel Barkalow <barkalow@iabervon.org>
Index: Makefile
===================================================================
--- ed4f6e454b40650b904ab72048b2f93a068dccc3/Makefile (mode:100644 sha1:b39b4ea37586693dd707d1d0750a9b580350ec50)
+++ d332a8ddffb50c1247491181af458970bf639942/Makefile (mode:100644 sha1:ca5dfd41b750cb1339128e4431afbbbc21bf57bb)
@@ -14,7 +14,7 @@
PROG= update-cache show-diff init-db write-tree read-tree commit-tree \
cat-file fsck-cache checkout-cache diff-tree rev-tree show-files \
- check-files ls-tree merge-tree
+ check-files ls-tree merge-tree http-get
all: $(PROG)
@@ -23,6 +23,11 @@
LIBS= -lssl -lz
+http-get: LIBS += -lcurl
+
+http-get:%:%.o read-cache.o
+ $(CC) $(CFLAGS) -o $@ $^ $(LIBS)
+
init-db: init-db.o
update-cache: update-cache.o read-cache.o
Index: http-get.c
===================================================================
--- /dev/null (tree:ed4f6e454b40650b904ab72048b2f93a068dccc3)
+++ d332a8ddffb50c1247491181af458970bf639942/http-get.c (mode:100644 sha1:106ca31239e6afe6784e7c592234406f5c149e44)
@@ -0,0 +1,126 @@
+#include <fcntl.h>
+#include <unistd.h>
+#include <string.h>
+#include <stdlib.h>
+#include "cache.h"
+#include "revision.h"
+#include <errno.h>
+#include <stdio.h>
+
+#include <curl/curl.h>
+#include <curl/easy.h>
+
+static CURL *curl;
+
+static char *base;
+
+static int fetch(unsigned char *sha1)
+{
+ char *hex = sha1_to_hex(sha1);
+ char *filename = sha1_file_name(sha1);
+
+ char *url;
+ char *posn;
+ FILE *local;
+ struct stat st;
+
+ if (!stat(filename, &st)) {
+ return 0;
+ }
+
+ local = fopen(filename, "w");
+
+ if (!local) {
+ fprintf(stderr, "Couldn't open %s\n", filename);
+ return -1;
+ }
+
+ curl_easy_setopt(curl, CURLOPT_FILE, local);
+
+ url = malloc(strlen(base) + 50);
+ strcpy(url, base);
+ posn = url + strlen(base);
+ strcpy(posn, "objects/");
+ posn += 8;
+ memcpy(posn, hex, 2);
+ posn += 2;
+ *(posn++) = '/';
+ strcpy(posn, hex + 2);
+
+ curl_easy_setopt(curl, CURLOPT_URL, url);
+
+ curl_easy_perform(curl);
+
+ fclose(local);
+
+ return 0;
+}
+
+static int process_tree(unsigned char *sha1)
+{
+ void *buffer;
+ unsigned long size;
+ char type[20];
+
+ buffer = read_sha1_file(sha1, type, &size);
+ if (!buffer)
+ return -1;
+ if (strcmp(type, "tree"))
+ return -1;
+ while (size) {
+ int len = strlen(buffer) + 1;
+ unsigned char *sha1 = buffer + len;
+ unsigned int mode;
+ int retval;
+
+ if (size < len + 20 || sscanf(buffer, "%o", &mode) != 1)
+ return -1;
+
+ buffer = sha1 + 20;
+ size -= len + 20;
+
+ retval = fetch(sha1);
+ if (retval)
+ return -1;
+
+ if (S_ISDIR(mode)) {
+ retval = process_tree(sha1);
+ if (retval)
+ return -1;
+ }
+ }
+ return 0;
+}
+
+static int process_commit(unsigned char *sha1)
+{
+ struct revision *rev = lookup_rev(sha1);
+ if (parse_commit_object(rev))
+ return -1;
+
+ fetch(rev->tree);
+ process_tree(rev->tree);
+ return 0;
+}
+
+int main(int argc, char **argv)
+{
+ char *commit_id = argv[1];
+ char *url = argv[2];
+
+ unsigned char sha1[20];
+
+ get_sha1_hex(commit_id, sha1);
+
+ curl_global_init(CURL_GLOBAL_ALL);
+
+ curl = curl_easy_init();
+
+ base = url;
+
+ fetch(sha1);
+ process_commit(sha1);
+
+ curl_global_cleanup();
+ return 0;
+}
Index: revision.h
===================================================================
--- ed4f6e454b40650b904ab72048b2f93a068dccc3/revision.h (mode:100664 sha1:28d0de3261a61f68e4e0948a25a416a515cd2e83)
+++ d332a8ddffb50c1247491181af458970bf639942/revision.h (mode:100664 sha1:523bde6e14e18bb0ecbded8f83ad4df93fc467ab)
@@ -24,6 +24,7 @@
unsigned int flags;
unsigned char sha1[20];
unsigned long date;
+ unsigned char tree[20];
struct parent *parent;
};
@@ -111,4 +112,29 @@
}
}
+static int parse_commit_object(struct revision *rev)
+{
+ if (!(rev->flags & SEEN)) {
+ void *buffer, *bufptr;
+ unsigned long size;
+ char type[20];
+ unsigned char parent[20];
+
+ rev->flags |= SEEN;
+ buffer = bufptr = read_sha1_file(rev->sha1, type, &size);
+ if (!buffer || strcmp(type, "commit"))
+ return -1;
+ get_sha1_hex(bufptr + 5, rev->tree);
+ bufptr += 46; /* "tree " + "hex sha1" + "\n" */
+ while (!memcmp(bufptr, "parent ", 7) &&
+ !get_sha1_hex(bufptr+7, parent)) {
+ add_relationship(rev, parent);
+ bufptr += 48; /* "parent " + "hex sha1" + "\n" */
+ }
+ //rev->date = parse_commit_date(bufptr);
+ free(buffer);
+ }
+ return 0;
+}
+
#endif /* REVISION_H */
^ permalink raw reply
* Re: [PATCH] Use libcurl to use HTTP to get repositories
From: Paul Jackson @ 2005-04-17 0:29 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: git, torvalds
In-Reply-To: <Pine.LNX.4.21.0504162008190.30848-100000@iabervon.org>
> Needs libcurl post 7.7 or so.
That could be mentioned in the README, which has a list of 'Software
requirements.' Actually, zlib-devel and openssl should be on this list
as well. My laziness got in the way of my sending in a patch for that.
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <pj@engr.sgi.com> 1.650.933.1373, 1.925.600.0401
^ permalink raw reply
* Re: fix mktemp (remove mktemp ;)
From: Dave Jones @ 2005-04-17 0:33 UTC (permalink / raw)
To: Paul Jackson; +Cc: Petr Baudis, git, mj
In-Reply-To: <20050416170221.38b3e66c.pj@sgi.com>
On Sat, Apr 16, 2005 at 05:02:21PM -0700, Paul Jackson wrote:
> > And racy. And not guaranteed to come up with fresh new files.
>
> In theory perhaps. In practice no.
>
> Even mktemp(1) can collide, in theory, since there is no practical way
> in shell scripts to hold open and locked the file from the instant of it
> is determined to be a unique name.
Using the pid as a 'random' number is a bad idea. all an attacker
has to do is create 65535 symlinks in /usr/tmp, and he can now
overwrite any file you own.
mktemp is being used here to provide randomness in the filename,
not just a uniqueness.
> The window of vulnerability for shell script tmp files is the lifetime
> of the script - while the file sits there unlocked. Anyone else with
> permissions can mess with it.
Attacker doesnt need to touch the script. Just take advantage of
flaws in it, and wait for someone to run it.
> More people will fail, and are already failing, using mktemp than I have
> ever seen using $$ (I've never seen a documented case, and since such
> files are not writable to other user accounts, such a collision would
> typically not go hidden.)
>
> Fast, simple portable solutions that work win over solutions with some
> theoretical advantage that don't matter in practice, but also that are
> less portable or less efficient.
I'd suggest fixing your distributions mktemp over going with an
inferior solution.
Dave
^ permalink raw reply
* [PATCH] show-diff shell safety
From: Junio C Hamano @ 2005-04-17 0:36 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git
The command line for running "diff" command is built without
taking shell metacharacters into account. A malicious dircache
entry "foo 2>bar" (yes, a filename with space) would result in
creating a file called "bar" with the error message "diff: foo:
No such file or directory" in it.
This is not just a user screwing over himself. Such a dircache
can be created as a result of a merge with tree from others.
Here is a fix.
To be applied on top of my previous patches:
[PATCH] Optionally tell show-diff to show only named files.
[PATCH] show-diff -z option for machine readable output.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
show-diff.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 60 insertions(+), 4 deletions(-)
show-diff.c: cb1b7eaa9758e94a179e73a80b4b52ee7c34ca5d
--- show-diff.c
+++ show-diff.c 2005-04-16 17:25:22.000000000 -0700
@@ -4,14 +4,70 @@
* Copyright (C) Linus Torvalds, 2005
*/
#include "cache.h"
+#include <ctype.h>
-static void show_differences(char *name,
- void *old_contents, unsigned long long old_size)
+static char *diff_cmd = "diff -L '%s' -u -N - '%s'";
+
+/* Help to copy the thing properly quoted for the shell safety.
+ * any single quote is replaced with '\'', and the caller is
+ * expected to enclose the result within a single quote pair.
+ *
+ * E.g.
+ * original sq_expand result
+ * name ==> name ==> 'name'
+ * a b ==> a b ==> 'a b'
+ * a'b ==> a'\''b ==> 'a'\''b'
+ *
+ * NOTE! The returned memory belongs to this function so
+ * do not free it.
+ */
+static char *sq_expand(char *src)
+{
+ static char *buf = NULL;
+ static int buf_size = -1;
+ int cnt, c;
+ char *cp;
+
+ /* count single quote characters */
+ for (cnt = 0, cp = src; *cp; cnt++, cp++)
+ if (*cp == '\'')
+ cnt += 3;
+
+ if (buf_size < cnt) {
+ free(buf);
+ buf_size = cnt;
+ buf = malloc(cnt);
+ }
+
+ cp = buf;
+ while ((c = *src++)) {
+ if (c != '\'')
+ *cp++ = c;
+ else {
+ cp = strcpy(cp, "'\\''");
+ cp += 4;
+ }
+ }
+ *cp = 0;
+ return buf;
+}
+
+static void show_differences(char *name, void *old_contents,
+ unsigned long long old_size)
{
- static char cmd[1000];
FILE *f;
+ static char *cmd = NULL;
+ static int cmd_size = -1;
+
+ char *name_sq = sq_expand(name);
+ int cmd_required_length = strlen(name_sq) * 2 + strlen(diff_cmd);
- snprintf(cmd, sizeof(cmd), "diff -L %s -u -N - %s", name, name);
+ if (cmd_size < cmd_required_length) {
+ free(cmd);
+ cmd_size = cmd_required_length;
+ cmd = malloc(cmd_required_length);
+ }
+ snprintf(cmd, cmd_size, diff_cmd, name_sq, name_sq);
f = popen(cmd, "w");
if (old_size)
fwrite(old_contents, old_size, 1, f);
^ permalink raw reply
* Re: [PATCH] Use libcurl to use HTTP to get repositories
From: Daniel Barkalow @ 2005-04-17 0:42 UTC (permalink / raw)
To: Paul Jackson; +Cc: git, Linus Torvalds, Petr Baudis
In-Reply-To: <20050416172952.3bf34bab.pj@sgi.com>
On Sat, 16 Apr 2005, Paul Jackson wrote:
> > Needs libcurl post 7.7 or so.
>
> That could be mentioned in the README, which has a list of 'Software
> requirements.' Actually, zlib-devel and openssl should be on this list
> as well. My laziness got in the way of my sending in a patch for that.
I'm working off of Linus's tree when not working on scripts, and it
doesn't have that section at all. Should I submit patches to the pasky
tree? (I'm actually tempted to have my own public tree, now that I
wouldn't have to set up anonymous rsync access, aside from the
bootstrapping issue; then I could let people pull and merge instead of
applying patches.)
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: fix mktemp (remove mktemp ;)
From: Paul Jackson @ 2005-04-17 0:44 UTC (permalink / raw)
To: Dave Jones; +Cc: pasky, git, mj
In-Reply-To: <20050417003325.GA15608@redhat.com>
Dave wrote:
> mktemp is being used here to provide randomness in the filename,
> not just a uniqueness.
Ok - useful point.
How about:
t=${TMPDIR:-/usr/tmp}/gitdiff.$$.$RANDOM
> all an attacker has to do is create 65535 symlinks in /usr/tmp
And how about if I removed the tmp files at the top:
t=${TMPDIR:-/usr/tmp}/gitdiff.$$.$RANDOM
trap 'rm -fr $t.?; trap 0; exit 0' 0 1 2 3 15
rm -fr $t.?
... rest of script ...
How close does that come to providing the same level of safety, while
remaining portable over a wider range of systems, and not requiring that
a separate command be forked?
> I'd suggest fixing your distributions ...
It's not just my distro; it's the distros of all git users.
If apps can avoid depending on inessential details of their
environment, that's friendlier to all concerned.
And actually my distro is fine - it's just that I am running an old
version of it on one of my systems. Newer versions of the mktemp -t
option.
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <pj@engr.sgi.com> 1.650.933.1373, 1.925.600.0401
^ permalink raw reply
* Re: [PATCH] Use libcurl to use HTTP to get repositories
From: Paul Jackson @ 2005-04-17 0:45 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: git, torvalds, pasky
In-Reply-To: <Pine.LNX.4.21.0504162033060.30848-100000@iabervon.org>
Daniel wrote:
> I'm working off of Linus's tree when not working on scripts, and it
> doesn't have that section at all.
Ah so - nevermind my README comments then.
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <pj@engr.sgi.com> 1.650.933.1373, 1.925.600.0401
^ permalink raw reply
* Re: fix mktemp (remove mktemp ;)
From: Erik van Konijnenburg @ 2005-04-17 0:51 UTC (permalink / raw)
To: Dave Jones; +Cc: Paul Jackson, Petr Baudis, git, mj
In-Reply-To: <20050417003325.GA15608@redhat.com>
On Sat, Apr 16, 2005 at 08:33:25PM -0400, Dave Jones wrote:
> On Sat, Apr 16, 2005 at 05:02:21PM -0700, Paul Jackson wrote:
> > > And racy. And not guaranteed to come up with fresh new files.
> >
> > In theory perhaps. In practice no.
> >
> > Even mktemp(1) can collide, in theory, since there is no practical way
> > in shell scripts to hold open and locked the file from the instant of it
> > is determined to be a unique name.
>
> Using the pid as a 'random' number is a bad idea. all an attacker
> has to do is create 65535 symlinks in /usr/tmp, and he can now
> overwrite any file you own.
>
> mktemp is being used here to provide randomness in the filename,
> not just a uniqueness.
How about putting using .git/tmp.$$ or similar as tempfile?
This should satisfy both the portability and security requirements,
since the warnings against using $$ only apply to public directories.
Regards,
Erik
^ permalink raw reply
* Re: fix mktemp (remove mktemp ;)
From: Dave Jones @ 2005-04-17 0:57 UTC (permalink / raw)
To: Paul Jackson; +Cc: pasky, git, mj
In-Reply-To: <20050416174409.59f94c26.pj@sgi.com>
On Sat, Apr 16, 2005 at 05:44:09PM -0700, Paul Jackson wrote:
> Dave wrote:
> > mktemp is being used here to provide randomness in the filename,
> > not just a uniqueness.
>
> Ok - useful point.
>
> How about:
>
> t=${TMPDIR:-/usr/tmp}/gitdiff.$$.$RANDOM
pid is still predictable by watching ps output, $RANDOM is one of 32768
numbers, so it's still feasable to predict the result.
$RANDOM$RANDOM is better, and gets a little closer to mktemp strength randomness.
> > all an attacker has to do is create 65535 symlinks in /usr/tmp
> And how about if I removed the tmp files at the top:
>
> t=${TMPDIR:-/usr/tmp}/gitdiff.$$.$RANDOM
> trap 'rm -fr $t.?; trap 0; exit 0' 0 1 2 3 15
> rm -fr $t.?
>
> ... rest of script ...
Racy, though the chance of creating x thousand symlinks in such a small
window probably makes it a non-issue.
Actually.. http://www.linuxsecurity.com/content/view/115462/151/
has some interesting bits on temp dir creation without mktemp.
See section 3.4 onwards.
Dave
^ permalink raw reply
* Re: [PATCH] show-diff shell safety
From: Paul Jackson @ 2005-04-17 1:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: torvalds, git
In-Reply-To: <7vbr8e44u1.fsf@assigned-by-dhcp.cox.net>
Junio wrote:
> The command line for running "diff" command is built without
> taking shell metacharacters into account.
Ack - you're right. One should avoid popen and system
in all but personal hacking code. There are many ways,
beyond just embedded shell redirection, to cause problems
with these calls.
One should directly code execve(), execv(), or execl().
Search for "popen system IFS PATH"
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <pj@engr.sgi.com> 1.650.933.1373, 1.925.600.0401
^ permalink raw reply
* Re: Storing permissions
From: Morten Welinder @ 2005-04-17 1:01 UTC (permalink / raw)
To: Martin Mares; +Cc: git
In-Reply-To: <20050416230058.GA10983@ucw.cz>
> Does it really make sense to store full permissions in the trees? I think
> that remembering the x-bit should be good enough for almost all purposes
> and the other permissions should be left to the local environment.
It makes some sense in principle, but without storing what they mean
(i.e., group==?) it certainly makes no sense. It's a bit like unpacking a
tar file.
I suspect a non-readable file would cause a bit of a problem in the low-level
commands.
Morten
^ permalink raw reply
* Re: fix mktemp (remove mktemp ;)
From: David Lang @ 2005-04-17 1:03 UTC (permalink / raw)
To: Dave Jones; +Cc: Paul Jackson, pasky, git, mj
In-Reply-To: <20050417005757.GB15608@redhat.com>
set your umask to make things only writeable by the same user.
then create a new directory (it will fail with an error if the directory
already exists)
now you can create files in this directory without having to worry about
other users makeing trouble for you (they can't create symlinks in this
directory)
David Lang
On Sat, 16 Apr 2005, Dave Jones wrote:
> Date: Sat, 16 Apr 2005 20:57:57 -0400
> From: Dave Jones <davej@redhat.com>
> To: Paul Jackson <pj@sgi.com>
> Cc: pasky@ucw.cz, git@vger.kernel.org, mj@ucw.cz
> Subject: Re: fix mktemp (remove mktemp ;)
>
> On Sat, Apr 16, 2005 at 05:44:09PM -0700, Paul Jackson wrote:
> > Dave wrote:
> > > mktemp is being used here to provide randomness in the filename,
> > > not just a uniqueness.
> >
> > Ok - useful point.
> >
> > How about:
> >
> > t=${TMPDIR:-/usr/tmp}/gitdiff.$$.$RANDOM
>
> pid is still predictable by watching ps output, $RANDOM is one of 32768
> numbers, so it's still feasable to predict the result.
> $RANDOM$RANDOM is better, and gets a little closer to mktemp strength randomness.
>
> > > all an attacker has to do is create 65535 symlinks in /usr/tmp
> > And how about if I removed the tmp files at the top:
> >
> > t=${TMPDIR:-/usr/tmp}/gitdiff.$$.$RANDOM
> > trap 'rm -fr $t.?; trap 0; exit 0' 0 1 2 3 15
> > rm -fr $t.?
> >
> > ... rest of script ...
>
> Racy, though the chance of creating x thousand symlinks in such a small
> window probably makes it a non-issue.
>
> Actually.. http://www.linuxsecurity.com/content/view/115462/151/
> has some interesting bits on temp dir creation without mktemp.
> See section 3.4 onwards.
>
> Dave
>
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
There are two ways of constructing a software design. One way is to make it so simple that there are obviously no deficiencies. And the other way is to make it so complicated that there are no obvious deficiencies.
-- C.A.R. Hoare
^ permalink raw reply
* [PATCH] Rename confusing variable in show-diff
From: Junio C Hamano @ 2005-04-17 1:10 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git
The show-diff command uses a variable "new" but it is always
used to point at the original data recorded in the dircache
before the user started editing in the working file. Rename it
to "old" to avoid confusion.
To be applied on top of my previous patches:
[PATCH] Optionally tell show-diff to show only named files.
[PATCH] show-diff -z option for machine readable output.
[PATCH] show-diff shell safety.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
show-diff.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
show-diff.c: e52eee21c2f682bef2dba06445699cca8e34c63a
--- show-diff.c
+++ show-diff.c 2005-04-16 18:05:55.000000000 -0700
@@ -162,7 +162,7 @@
int changed;
unsigned long size;
char type[20];
- void *new;
+ void *old;
if (1 <argc &&
! matches_pathspec(ce, argv+1, argc-1))
@@ -193,8 +193,8 @@
if (silent)
continue;
- new = read_sha1_file(ce->sha1, type, &size);
- show_differences(ce->name, new, size);
+ old = read_sha1_file(ce->sha1, type, &size);
+ show_differences(ce->name, old, size);
free(new);
}
return 0;
^ permalink raw reply
* Re: fix mktemp (remove mktemp ;)
From: Paul Jackson @ 2005-04-17 1:15 UTC (permalink / raw)
To: Dave Jones; +Cc: pasky, git, mj
In-Reply-To: <20050417005757.GB15608@redhat.com>
Dave wrote:
> http://www.linuxsecurity.com/content/view/115462/151/
Nice - thanks.
Pasky - would you be interested in a patch that used a more robust tmp
file creation, along the lines of replacing
t=${TMPDIR:-/usr/tmp}/gitdiff.$$
trap 'set +f; rm -fr $t.?; trap 0; exit 0' 0 1 2 3 15
with:
tmp=${TMPDIR-/tmp}
tmp=$tmp/gitdiff-do.$RANDOM.$RANDOM.$RANDOM.$$
(umask 077 && mkdir $tmp) || {
echo "Could not create temporary directory! Exiting." 1>&2
exit 1
}
t=$tmp/tmp
trap 'rm -fr $tmp; trap 0; exit 0' 0 1 2 3 15
If interested, would you want it instead of my previous mktemp removal
patch, or on top of it?
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <pj@engr.sgi.com> 1.650.933.1373, 1.925.600.0401
^ permalink raw reply
* Re: fix mktemp (remove mktemp ;)
From: Paul Jackson @ 2005-04-17 1:18 UTC (permalink / raw)
To: Erik van Konijnenburg; +Cc: davej, pasky, git, mj
In-Reply-To: <20050417025108.G2442@banaan.localdomain>
Erik wrote:
> How about putting using .git/tmp.$$ or similar as tempfile?
One could, but best to normally honor the users TMPDIR setting.
Could one 'git diff' a readonly git repository?
Perhaps someone has a reason for putting their tmp files where
they choose - say a local file system in a heavy NFS environment.
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <pj@engr.sgi.com> 1.650.933.1373, 1.925.600.0401
^ permalink raw reply
* [PATCH] (take 2) Rename confusing variable in show-diff
From: Junio C Hamano @ 2005-04-17 1:26 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git
In-Reply-To: <7vzmvy2ooq.fsf@assigned-by-dhcp.cox.net>
Oops, sorry I screwed up and sent a wrong patch. Please discard
the previous one.
The show-diff command uses a variable "new" but it is always
used to point at the original data recorded in the dircache
before the user started editing in the working file. Rename it
to "old" to avoid confusion.
To be applied on top of my previous patches:
[PATCH] Optionally tell show-diff to show only named files.
[PATCH] show-diff -z option for machine readable output.
[PATCH] show-diff shell safety.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
show-diff.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
show-diff.c: e52eee21c2f682bef2dba06445699cca8e34c63a
--- show-diff.c
+++ show-diff.c 2005-04-16 18:23:57.000000000 -0700
@@ -162,7 +162,7 @@
int changed;
unsigned long size;
char type[20];
- void *new;
+ void *old;
if (1 <argc &&
! matches_pathspec(ce, argv+1, argc-1))
@@ -193,9 +193,9 @@
if (silent)
continue;
- new = read_sha1_file(ce->sha1, type, &size);
- show_differences(ce->name, new, size);
- free(new);
+ old = read_sha1_file(ce->sha1, type, &size);
+ show_differences(ce->name, old, size);
+ free(old);
}
return 0;
}
^ permalink raw reply
* Re: Storing permissions
From: Paul Jackson @ 2005-04-17 1:30 UTC (permalink / raw)
To: Morten Welinder; +Cc: mj, git
In-Reply-To: <118833cc05041618017fb32a2@mail.gmail.com>
Morten wrote:
> It makes some sense in principle, but without storing what they mean
> (i.e., group==?) it certainly makes no sense.
There's no "they" there.
I think Martin's proposal, to which I agreed, was to store a _single_
bit. If any of the execute permissions of the incoming file are set,
then the bit is stored ON, else it is stored OFF. On 'checkout', if the
bit is ON, then the file permission is set mode 0777 (modulo umask),
else it is set mode 0666 (modulo umask).
You might disagree that this is a good idea, but it certainly does
'make sense' (as in 'is sensibly well defined').
> I suspect a non-readable file would cause a bit of a problem in the low-level
> commands.
Probably so. If someone sets their umask 0333 or less, then they are
either fools or QA (software quality assurance, or test) engineers.
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <pj@engr.sgi.com> 1.650.933.1373, 1.925.600.0401
^ permalink raw reply
* [PATCH] show-diff style fix.
From: Junio C Hamano @ 2005-04-17 2:15 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git
This fixes some stylistic problems introduced by my previous set
of patches. I'll be sending my last patch to show-diff next,
which depends on this cleanup.
To be applied on top of my previous patches:
[PATCH] Optionally tell show-diff to show only named files.
[PATCH] show-diff -z option for machine readable output.
[PATCH] show-diff shell safety.
[PATCH] (take 2) Rename confusing variable in show-diff.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
show-diff.c | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)
--- ./show-diff.c 2005-04-16 18:59:09.000000000 -0700
+++ ./show-diff.c 2005-04-16 19:01:28.000000000 -0700
@@ -111,7 +111,7 @@
}
}
-static const char *show_diff_usage = "show-diff [-s] [-q] [-z] [paths...]";
+static const char *show_diff_usage = "show-diff [-q] [-s] [-z] [paths...]";
static int matches_pathspec(struct cache_entry *ce, char **spec, int cnt)
{
@@ -141,9 +141,8 @@
silent_on_nonexisting_files = silent = 1;
else if (!strcmp(argv[1], "-q"))
silent_on_nonexisting_files = 1;
- else if (!strcmp(argv[1], "-z")) {
+ else if (!strcmp(argv[1], "-z"))
machine_readable = 1;
- }
else
usage(show_diff_usage);
argv++; argc--;
@@ -164,7 +163,7 @@
char type[20];
void *old;
- if (1 <argc &&
+ if (1 < argc &&
! matches_pathspec(ce, argv+1, argc-1))
continue;
^ permalink raw reply
* [PATCH] Do not run useless show-diff on unmerged paths repeatedly.
From: Junio C Hamano @ 2005-04-17 2:18 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git
When run on unmerged dircache, show-diff compares the working
file with each non-empty stage for that path. Two out of three
times, this is not very helpful. This patch makes it report the
unmergedness only once per each path and avoids running the
actual diff.
Upper layer SCMs like Cogito are expected to find out mode/SHA1
for each stage by using "show-files --stage" and run the diff
itself. This would result in more sensible diffs.
To be applied on top of my previous patches:
[PATCH] Optionally tell show-diff to show only named files.
[PATCH] show-diff -z option for machine readable output.
[PATCH] show-diff shell safety.
[PATCH] (take 2) Rename confusing variable in show-diff.
[PATCH] show-diff style fix.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
++show-diff.c | 13 +++++++++++++
1 files changed, 13 insertions(+)
--- ./show-diff.c 2005-04-16 19:01:28.000000000 -0700
+++ ./++show-diff.c 2005-04-16 18:57:55.000000000 -0700
@@ -167,6 +167,19 @@
! matches_pathspec(ce, argv+1, argc-1))
continue;
+ if (ce_stage(ce)) {
+ if (machine_readable)
+ printf("U %s%c", ce->name, 0);
+ else
+ printf("%s: Unmerged\n",
+ ce->name);
+ while (i < entries &&
+ !strcmp(ce->name, active_cache[i]->name))
+ i++;
+ i--; /* compensate for loop control increments */
+ continue;
+ }
+
if (stat(ce->name, &st) < 0) {
if (errno == ENOENT && silent_on_nonexisting_files)
continue;
^ permalink raw reply
* Re: fix mktemp (remove mktemp ;)
From: Brian O'Mahoney @ 2005-04-17 2:38 UTC (permalink / raw)
To: Paul Jackson; +Cc: Dave Jones, pasky, git, mj
In-Reply-To: <20050416174409.59f94c26.pj@sgi.com>
No, you have to:
(a) create a unique, pid specific file name /var/tmp/myapp.$$.xyzzy
(b) create it in O_EXCL mode, so you wont smash another's held lock
(b-1) It worked, OK
(b-2) open failed, try ...xyzzz
repeat until (b-1)
There are thousands of examples of how to do this with bash.
Paul Jackson wrote:
> Dave wrote:
>
>>mktemp is being used here to provide randomness in the filename,
>>not just a uniqueness.
>
>
> Ok - useful point.
>
> How about:
>
> t=${TMPDIR:-/usr/tmp}/gitdiff.$$.$RANDOM
>
>
>>all an attacker has to do is create 65535 symlinks in /usr/tmp
the point of the xyzzy seed is to make creating all possible files
in-feasable.
>
>
> And how about if I removed the tmp files at the top:
>
> t=${TMPDIR:-/usr/tmp}/gitdiff.$$.$RANDOM
> trap 'rm -fr $t.?; trap 0; exit 0' 0 1 2 3 15
> rm -fr $t.?
>
> ... rest of script ...
>
> How close does that come to providing the same level of safety, while
> remaining portable over a wider range of systems, and not requiring that
> a separate command be forked?
>
>
>>I'd suggest fixing your distributions ...
>
>
> It's not just my distro; it's the distros of all git users.
>
> If apps can avoid depending on inessential details of their
> environment, that's friendlier to all concerned.
>
> And actually my distro is fine - it's just that I am running an old
> version of it on one of my systems. Newer versions of the mktemp -t
> option.
>
--
mit freundlichen Grüßen, Brian.
Dr. Brian O'Mahoney
Mobile +41 (0)79 334 8035 Email: omb@bluewin.ch
Bleicherstrasse 25, CH-8953 Dietikon, Switzerland
PGP Key fingerprint = 33 41 A2 DE 35 7C CE 5D F5 14 39 C9 6D 38 56 D5
^ permalink raw reply
* Re: fix mktemp (remove mktemp ;)
From: Paul Jackson @ 2005-04-17 2:46 UTC (permalink / raw)
To: omb; +Cc: omb, davej, pasky, git, mj
In-Reply-To: <4261CC35.4070108@khandalf.com>
> No, you have to:
How does this compare with the one I posted about 1 hour 30
minuts ago:
tmp=${TMPDIR-/tmp}
tmp=$tmp/gitdiff-do.$RANDOM.$RANDOM.$RANDOM.$$
(umask 077 && mkdir $tmp) || {
echo "Could not create temporary directory! Exiting." 1>&2
exit 1
}
t=$tmp/tmp
trap 'rm -fr $tmp; trap 0; exit 0' 0 1 2 3 15
derived from the reference that Dave Jones provided?
> create it in O_EXCL mode,
What can one do that and hold that O_EXCL from within bash?
> There are thousands of examples of how to do this with bash.
Care to provide one?
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <pj@engr.sgi.com> 1.650.933.1373, 1.925.600.0401
^ permalink raw reply
* [PATCH] libgit
From: Mike Taht @ 2005-04-17 3:12 UTC (permalink / raw)
To: git
commit b0550573055abcf8ad19dcb8a036c32dd00a3be4
tree b77882b170769c07732381b9f19ff2dd5c9f1520
parent 866b4aea9313513612f2b0d66814a2f526d17f21
author Mike Taht <m@picketwyre.com> 1113704772 -0700
committer Mike Taht <m@ipbx.taht.net> 1113704772 -0700
looks my 1878 line patch to convert git to libgit got eaten by vger..
I put it up at http://pbx.picketwyre.com/~mtaht/libgit.patch if anyone
wants to comment. from my log:
Converted git to libgit. Moved all the main() calls into a single
multi-call binary - git-main.
Made extern a bunch of functions that were static. Verified it at least
still minimally worked.
Note: this is only a first step towards creating a generic library.
Figuring out what functions and variables *truly* need to be exported,
renaming them to a git_<function> api, making it thread safe
... and not least of all, keeping up with everybody working out of the
base tree... are problems that remain. Also - cleaning up the UI.
^ permalink raw reply
* Re: [PATCH] Get commits from remote repositories by HTTP
From: tony.luck @ 2005-04-17 3:16 UTC (permalink / raw)
To: Adam Kropelin; +Cc: Daniel Barkalow, git
In-Reply-To: <011201c542d5$940bb670$03c8a8c0@kroptech.com>
>How about building a file list and doing a batch download via 'wget -i
>/tmp/foo'? A quick test (on my ancient wget-1.7) indicates that it reuses
>connectionss when successive URLs point to the same server.
Here's a script that does just that. So there is a burst of individual
wget commands to get HEAD, the top commit object, and all the tree
objects. The just one to get all the missing blobs.
Subsequent runs will do far less work as many of the tree objects will
not have changed, so we don't descend into any tree that we already have.
-Tony
Not a patch ... it is a whole file. I called it "git-wget", but it might
also want to be called "git-pulltop".
Signed-off-by: Tony Luck <tony.luck@intel.com>
------ script starts here -----
#!/bin/sh
# Copyright (C) 2005 Tony Luck
REMOTE=http://www.kernel.org/pub/linux/kernel/people/torvalds/linux-2.6.git/
rm -rf .gittmp
# set up a temp git repository so that we can use cat-file and ls-tree on the
# objects we pull without installing them into our tree. This allows us to
# restart if the download is interrupted
mkdir .gittmp
cd .gittmp
init-db
wget -q $REMOTE/HEAD
if cmp -s ../.git/HEAD HEAD
then
echo Already have HEAD = `cat ../.git/HEAD`
cd ..
rm -rf .gittmp
exit 0
fi
sha1=`cat HEAD`
sha1file=${sha1:0:2}/${sha1:2}
if [ -f ../.git/objects/$sha1file ]
then
echo Already have most recent commit. Update HEAD to $sha1
cd ..
rm -rf .gittmp
exit 0
fi
wget -q $REMOTE/objects/$sha1file -O .git/objects/$sha1file
treesha1=`cat-file commit $sha1 | (read tag tree ; echo $tree)`
get_tree()
{
treesha1file=${1:0:2}/${1:2}
if [ -f ../.git/objects/$treesha1file ]
then
return
fi
wget -q $REMOTE/objects/$treesha1file -O .git/objects/$treesha1file
ls-tree $1 | while read mode tag sha1 name
do
subsha1file=${sha1:0:2}/${sha1:2}
if [ -f ../.git/objects/$subsha1file ]
then
continue
fi
if [ $mode = 40000 ]
then
get_tree $sha1 `expr $2 + 1`
else
echo objects/$subsha1file >> needbloblist
fi
done
}
# get all the tree objects to our .gittmp area, and create list of needed blobs
get_tree $treesha1
# now get the blobs
cd ../.git
if [ -s ../.gittmp/needbloblist ]
then
wget -q -r -nH --cut-dirs=6 --base=$REMOTE -i ../.gittmp/needbloblist
fi
# Now we have the blobs, move the trees and commit from .gitttmp
cd ../.gittmp/.git/objects
find ?? -type f -print | while read f
do
mv $f ../../../.git/objects/$f
done
# update HEAD
cd ../..
mv HEAD ../.git
cd ..
rm -rf .gittmp
------ script ends here -----
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox