* [PATCH/RFC] add lame win32 credential-helper
From: Erik Faye-Lund @ 2011-09-15 20:25 UTC (permalink / raw)
To: git; +Cc: jaysoffian, peff, gitster
Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
I got curious what a credential-helper that uses Windows'
Credential Manager would look like; this is the result.
Some parts of the code is heavily inspired by Jay Soffian's
OSX-keychain work.
Not that it's useful yet, since the core-git code for the
credential-helper support doesn't compile on Windows. So
it's not fully tested, I've only read the interface
documentation and experimented with it from the command
line.
contrib/credential-wincred/Makefile | 8 +
.../credential-wincred/git-credential-wincred.c | 271 ++++++++++++++++++++
2 files changed, 279 insertions(+), 0 deletions(-)
create mode 100644 contrib/credential-wincred/Makefile
create mode 100644 contrib/credential-wincred/git-credential-wincred.c
diff --git a/contrib/credential-wincred/Makefile b/contrib/credential-wincred/Makefile
new file mode 100644
index 0000000..b4f098f
--- /dev/null
+++ b/contrib/credential-wincred/Makefile
@@ -0,0 +1,8 @@
+all: git-credential-wincred.exe
+
+CC = gcc
+RM = rm -f
+CFLAGS = -O2 -Wall
+
+git-credential-wincred.exe : git-credential-wincred.c
+ $(LINK.c) $^ $(LOADLIBES) $(LDLIBS) -o $@
diff --git a/contrib/credential-wincred/git-credential-wincred.c b/contrib/credential-wincred/git-credential-wincred.c
new file mode 100644
index 0000000..5c0e2d3
--- /dev/null
+++ b/contrib/credential-wincred/git-credential-wincred.c
@@ -0,0 +1,271 @@
+#include <windows.h>
+#include <stdio.h>
+
+/* MinGW doesn't have wincred.h, let's extract the stuff we need instead */
+
+typedef struct _CREDENTIAL_ATTRIBUTE {
+ LPWSTR Keyword;
+ DWORD Flags;
+ DWORD ValueSize;
+ LPBYTE Value;
+} CREDENTIAL_ATTRIBUTE, *PCREDENTIAL_ATTRIBUTE;
+
+typedef struct _CREDENTIALW {
+ DWORD Flags;
+ DWORD Type;
+ LPWSTR TargetName;
+ LPWSTR Comment;
+ FILETIME LastWritten;
+ DWORD CredentialBlobSize;
+ LPBYTE CredentialBlob;
+ DWORD Persist;
+ DWORD AttributeCount;
+ PCREDENTIAL_ATTRIBUTE Attributes;
+ LPWSTR TargetAlias;
+ LPWSTR UserName;
+} CREDENTIALW, *PCREDENTIALW;
+
+typedef struct _CREDUI_INFOW {
+ DWORD cbSize;
+ HWND hwndParent;
+ LPWSTR pszMessageText;
+ LPWSTR pszCaptionText;
+ HBITMAP hbmBanner;
+} CREDUI_INFOW, *PCREDUI_INFOW;
+
+#define CRED_TYPE_GENERIC 1
+#define CRED_PERSIST_LOCAL_MACHINE 2
+#define CREDUIWIN_GENERIC 1
+#define CREDUIWIN_CHECKBOX 2
+#define CREDUIWIN_IN_CRED_ONLY 32
+#define CRED_PACK_GENERIC_CREDENTIALS 4
+
+
+typedef BOOL (WINAPI *CredWriteWT)(PCREDENTIALW, DWORD);
+typedef BOOL (WINAPI *CredUnPackAuthenticationBufferWT)(DWORD, PVOID, DWORD,
+ LPWSTR, DWORD *, LPWSTR, DWORD *, LPWSTR, DWORD *);
+typedef DWORD (WINAPI *CredUIPromptForWindowsCredentialsWT)(PCREDUI_INFOW,
+ DWORD, ULONG *, LPCVOID, ULONG, LPVOID *, ULONG *, BOOL *, DWORD);
+typedef BOOL (WINAPI *CredEnumerateWT)(LPCWSTR, DWORD, DWORD *,
+ PCREDENTIALW **);
+typedef BOOL (WINAPI *CredPackAuthenticationBufferWT)(DWORD, LPWSTR, LPWSTR,
+ PBYTE, DWORD *);
+typedef VOID (WINAPI *CredFreeT)(PVOID);
+typedef BOOL (WINAPI *CredDeleteWT)(LPCWSTR, DWORD, DWORD);
+
+static HMODULE advapi, credui;
+static CredWriteWT CredWriteW;
+static CredUnPackAuthenticationBufferWT CredUnPackAuthenticationBufferW;
+static CredUIPromptForWindowsCredentialsWT CredUIPromptForWindowsCredentialsW;
+static CredEnumerateWT CredEnumerateW;
+static CredPackAuthenticationBufferWT CredPackAuthenticationBufferW;
+static CredFreeT CredFree;
+static CredDeleteWT CredDeleteW;
+
+static void die(const char *err, ...)
+{
+ char msg[4096];
+ va_list params;
+ va_start(params, err);
+ vsnprintf(msg, sizeof(msg), err, params);
+ fprintf(stderr, "%s\n", msg);
+ va_end(params);
+ exit(1);
+}
+
+static void emit_user_pass(WCHAR *username, WCHAR *password)
+{
+ if (username)
+ wprintf(L"username=%s\n", username);
+ if (password)
+ wprintf(L"password=%s\n", password);
+}
+
+static int find_credentials(WCHAR *target, WCHAR *username)
+{
+ WCHAR user_buf[256], pass_buf[256];
+ DWORD user_buf_size = sizeof(user_buf) - 1,
+ pass_buf_size = sizeof(pass_buf) - 1;
+ CREDENTIALW **creds, *cred = NULL;
+ DWORD num_creds;
+
+ if (!CredEnumerateW(target, 0, &num_creds, &creds))
+ return -1;
+
+ if (!username) {
+ /* no username was specified, just pick the first one */
+ cred = creds[0];
+ } else {
+ /* search for the first credential that matches username */
+ int i;
+ for (i = 0; i < num_creds; ++i)
+ if (!wcscmp(username, creds[i]->UserName)) {
+ cred = creds[i];
+ break;
+ }
+ if (!cred)
+ return -1;
+ }
+
+ if (!CredUnPackAuthenticationBufferW(0, cred->CredentialBlob,
+ cred->CredentialBlobSize, user_buf, &user_buf_size, NULL, NULL,
+ pass_buf, &pass_buf_size))
+ return -1;
+
+ CredFree(creds);
+
+ /* zero terminate */
+ user_buf[user_buf_size] = L'\0';
+ pass_buf[pass_buf_size] = L'\0';
+
+ emit_user_pass(user_buf, pass_buf);
+ return 0;
+}
+
+/* also saves the credentials if the user tells it to */
+static int ask_credentials(WCHAR *target, WCHAR *comment, WCHAR *username)
+{
+ BOOL save = FALSE;
+ LPVOID auth_buf = NULL;
+ ULONG auth_buf_size = 0;
+ WCHAR user_buf[256], pass_buf[256];
+ DWORD user_buf_size = sizeof(user_buf) - 1,
+ pass_buf_size = sizeof(pass_buf) - 1;
+ BYTE in_buf[1024];
+ DWORD in_buf_size = sizeof(in_buf);
+ DWORD err;
+ ULONG package = 0;
+ CREDUI_INFOW info = {
+ sizeof(info), NULL,
+ comment ? comment : target, L"Enter password", NULL
+ };
+
+ if (username)
+ CredPackAuthenticationBufferW(0, username, L"",
+ in_buf, &in_buf_size);
+ err = CredUIPromptForWindowsCredentialsW(&info, 0, &package,
+ in_buf, in_buf_size, &auth_buf, &auth_buf_size,
+ &save, CREDUIWIN_GENERIC | CREDUIWIN_CHECKBOX);
+ if (err == ERROR_CANCELLED)
+ return 0;
+ if (err != ERROR_SUCCESS)
+ return -1;
+
+ if (!CredUnPackAuthenticationBufferW(0, auth_buf, auth_buf_size,
+ user_buf, &user_buf_size, NULL, NULL,
+ pass_buf, &pass_buf_size))
+ return -1;
+
+ /* zero terminate */
+ user_buf[user_buf_size] = L'\0';
+ pass_buf[pass_buf_size] = L'\0';
+
+ emit_user_pass(user_buf, pass_buf);
+
+ if (save) {
+ CREDENTIALW cred;
+ cred.Flags = 0;
+ cred.Type = CRED_TYPE_GENERIC;
+ cred.TargetName = target;
+ cred.Comment = comment;
+ cred.CredentialBlobSize = auth_buf_size;
+ cred.CredentialBlob = auth_buf;
+ cred.Persist = CRED_PERSIST_LOCAL_MACHINE;
+ cred.AttributeCount = 0;
+ cred.Attributes = NULL;
+ cred.TargetAlias = NULL;
+ cred.UserName = user_buf;
+ if (!CredWriteW(&cred, 0))
+ fprintf(stderr, "failed to write credentials\n");
+ }
+ return 0;
+}
+
+static void delete_credentials(WCHAR *target, WCHAR *username)
+{
+ WCHAR temp[4096];
+
+ wcscpy(temp, target);
+ if (username) {
+ wcscat(temp, L"|");
+ wcscat(temp, username);
+ }
+ if (!CredDeleteW(target, CRED_TYPE_GENERIC, 0))
+ die("failed to delete credentials");
+}
+
+int main(int argc, char *argv[])
+{
+ const char *usage =
+ "Usage: git credential-osxkeychain --unique=TOKEN [options]\n"
+ "Options:\n"
+ " --description=DESCRIPTION\n"
+ " --username=USERNAME\n"
+ " --reject";
+ WCHAR desc_buf[4096], *description = NULL,
+ user_buf[256], *username = NULL,
+ unique_buf[1024], *unique = NULL;
+ int i, reject = 0;
+
+ for (i = 1; i < argc; ++i) {
+ const char *arg = argv[i];
+ if (!strncmp(arg, "--description=", 14)) {
+ MultiByteToWideChar(CP_UTF8, 0, arg + 14, -1,
+ desc_buf, sizeof(desc_buf));
+ description = desc_buf;
+ } else if (!strncmp(arg, "--username=", 11)) {
+ MultiByteToWideChar(CP_UTF8, 0, arg + 11, -1,
+ user_buf, sizeof(user_buf));
+ username = user_buf;
+ } else if (!strncmp(arg, "--unique=", 9)) {
+ MultiByteToWideChar(CP_UTF8, 0, arg + 9, -1,
+ unique_buf, sizeof(unique_buf));
+ unique = unique_buf;
+ } else if (!strcmp(arg, "--reject")) {
+ reject = 1;
+ } else if (!strcmp(arg, "--help")) {
+ die(usage);
+ } else
+ die("Unrecognized argument `%s'; try --help", arg);
+ }
+
+ if (!unique)
+ die("Must specify --unique=TOKEN; try --help");
+
+ /* load DLLs */
+ advapi = LoadLibrary("advapi32.dll");
+ credui = LoadLibrary("credui.dll");
+ if (!advapi || !credui)
+ die("failed to load DLLs");
+
+ /* get function pointers */
+ CredWriteW = (CredWriteWT)GetProcAddress(advapi, "CredWriteW");
+ CredUnPackAuthenticationBufferW = (CredUnPackAuthenticationBufferWT)
+ GetProcAddress(credui, "CredUnPackAuthenticationBufferW");
+ CredUIPromptForWindowsCredentialsW =
+ (CredUIPromptForWindowsCredentialsWT)GetProcAddress(credui,
+ "CredUIPromptForWindowsCredentialsW");
+ CredEnumerateW = (CredEnumerateWT)GetProcAddress(advapi,
+ "CredEnumerateW");
+ CredPackAuthenticationBufferW = (CredPackAuthenticationBufferWT)
+ GetProcAddress(credui, "CredPackAuthenticationBufferW");
+ CredFree = (CredFreeT)GetProcAddress(advapi, "CredFree");
+ CredDeleteW = (CredDeleteWT)GetProcAddress(advapi, "CredDeleteW");
+ if (!CredWriteW || !CredUnPackAuthenticationBufferW ||
+ !CredUIPromptForWindowsCredentialsW || !CredEnumerateW ||
+ !CredPackAuthenticationBufferW || !CredFree || !CredDeleteW)
+ die("failed to load functions");
+
+ if (reject) {
+ delete_credentials(unique, username);
+ return 0;
+ }
+
+ if (!find_credentials(unique, username))
+ return 0;
+
+ if (!ask_credentials(unique, description, username))
+ return 0;
+
+ return -1;
+}
--
1.7.6.355.g842ba.dirty
^ permalink raw reply related
* Re: [PATCH 0/4] Honor core.ignorecase for attribute patterns
From: Brandon Casey @ 2011-09-15 20:28 UTC (permalink / raw)
To: Jeff King; +Cc: git, gitster, sunshine, bharrosh, trast, zapped
In-Reply-To: <20110915181258.GA1227@sigill.intra.peff.net>
On Thu, Sep 15, 2011 at 1:12 PM, Jeff King <peff@peff.net> wrote:
> On Wed, Sep 14, 2011 at 08:59:35PM -0500, Brandon Casey wrote:
>
>> > I haven't even tested that it runs. :) No, I was hoping someone
>> > who was more interested would finish it, and maybe even test on
>> > an affected system.
>>
>> Ok, I lied. Here's a series that needs testing by people on a
>> case-insensitive filesystem and some comments.
>
> Thanks. I was trying to decide if I was interested enough to work on it,
> but procrastination wins again.
>
> I'm not sure I understand why you need a case-insensitive file system
> for the final set of tests. If we have a case-sensitive system, we can
> force the filesystem to show us whatever cases we want, and check
> against them with both core.ignorecase off and on[1]. What are these
> tests checking that requires the actual behavior of a case-insensitive
> filesystem?
This is probably way more detail than this feature deserves, but...
Those tests are making sure that git handles the case where the
.gitignore file resides in a subdirectory and the user supplies a path
that does not match the case in the filesystem. In that
case^H^H^H^Hsituation, part of the path supplied by the user is
effectively interpreted case-insensitively, and part of it is
dependent on the setting of core.ignorecase. git should only be
matching the portion of the path below the directory holding the
.gitignore file according to the setting of core.ignorecase.
Imagine a hierarchy that looks like this:
.gitattributes
a/.gitattributes
On a case-insensitive filesystem, if you supply the path A/B,
regardless of whether ignorecase is true or false, git will read the
a/.gitattributes file and use it.
Then if you have:
$ cat a/.gitattributes
b/c test=a/b/c
then you should get the following results:
# the case of a/ does not affect the attr check
$ git -c core.ignorecase=0 check-attr a/b/c
a/b/c: test: a/b/c
$ git -c core.ignorecase=0 check-attr A/b/c
A/b/c: test: a/b/c
$ git -c core.ignorecase=0 check-attr a/B/c
a/B/c: test: unspecified
$ git -c core.ignorecase=1 check-attr a/B/c
a/B/c: test: a/b/c
$ git -c core.ignorecase=0 check-attr A/B/c
A/B/c: test: unspecified
$ git -c core.ignorecase=1 check-attr A/B/c
A/B/c: test: a/b/c
etc.
On a case-sensitive filesystem, a/.gitattributes would never be read
if A/b/c was supplied, regardless of core.ignorecase.
This is also partly future-proofing. Currently, git builds the attr
stack based on the path supplied by the user, so we don't have to do
anything special (like use strcmp_icase) to handle the parts of that
path that don't match the filesystem with respect to case. If git
instead built the attr stack by scanning the repository, then the
paths in the origin field would not necessarily match the paths
supplied by the user. If someone makes a change like that in the
future, these tests will notice.
> I'm sure there is something subtle that I'm missing. Can you explain it
> either here or in the commit message?
Yeah, that commit message was really just a place-holder. I meant to
add WIP in the subject field of the last patch too. I'll try to
explain some of the above when I reroll.
-Brandon
^ permalink raw reply
* Re: Problems with format-patch UTF-8 and a missing second empty line
From: Jeff King @ 2011-09-15 20:33 UTC (permalink / raw)
To: Alexey Shumkin; +Cc: Ingo Ruhnke, git
In-Reply-To: <20110916000515.1dfc5665@zappedws>
On Fri, Sep 16, 2011 at 12:05:15AM +0400, Alexey Shumkin wrote:
> But as you said
> >>This is by design. Git commit messages are intended to have a
> >>single-line subject, followed by a blank line, followed by more
> >>elaboration
>
> and solved with "-k" for both "format-patch" and "am" commands
OK, that makes sense to me, then. I didn't read Ingo's first message
carefully enough, but your response made me scratch my head and read it
again. Thanks for the sanity check.
-Peff
^ permalink raw reply
* Re: vcs-svn and friends
From: Jonathan Nieder @ 2011-09-15 20:48 UTC (permalink / raw)
To: Stephen Bash; +Cc: David Michael Barr, Dmitry Ivankov, git, Junio C Hamano
In-Reply-To: <27874151.17649.1316095596748.JavaMail.root@mail.hq.genarts.com>
Stephen Bash wrote:
> For those of us interested but out of the loop, does this mean you
> have a working example where I can point it at a SVN repo and see
> what happens? Having done our SVN to Git conversion last year, I
> know our repo has a lot of the common SVN screw cases (non-branching
> copies, partial merges, mis-merges, *lots* of retagging, changes
> committed to tags, etc.) so if it's relatively easy to setup a test
> I'm happy to run one.
Thanks. It's very bare-bones at the moment: it just imports each
revision as a whole tree, with no branch and merge tracking at all.
So it would be very interesting to get this basic stuff out into the
wild and then add some code implementing those things for you to break
on top of it.
^ permalink raw reply
* Re: Anybody home?
From: Joshua Stoutenburg @ 2011-09-15 21:00 UTC (permalink / raw)
To: Scott Chacon; +Cc: Git List
In-Reply-To: <CAP2yMaJcKnghtxxDNVYk=00KQ-ZOGEwACHRTU15tGnOuk3R4Hw@mail.gmail.com>
On Thu, Sep 15, 2011 at 8:04 AM, Scott Chacon <schacon@gmail.com> wrote:
> Hey,
>
> On Thu, Sep 15, 2011 at 2:01 AM, Joshua Stoutenburg
> <jehoshua02@gmail.com> wrote:
>>> Reading your exchanges elsewhere in this thread, I think you missed that
>>> you don't need a git server at all just to *use* git.
>>>
>>> Even when you want to exchange your commits between two or three machines,
>>> all you need is ssh access. There is no *git server* necessary. git is not
>>> svn. ;-)
>>>
>>> I thought I'd just mention this to help you streamline your search.
>>>
>>> -- Hannes
>>>
>>
>> I read the first four and a half chapters from the Pro Git book pdf.
>> So I think I understood that much.
>>
>> But in my situation, I do need a server so that other developers can
>> access anytime over the internet.
>>
>> I should have mentioned that.
>
> I guess I'm confused. The fourth chapter of the Pro Git book is
> entirely about setting up your own Git server, including basically
> step by step instructions on Gitolite and Gitosis, in addition to
> simply running your own ssh-based server plus gitweb. It is like 20
> pages long - how is this not exactly what you're asking for?
>
> Scott
>
Scott -- the man himself!
I've been reading the pdf version of the excellent Pro Git book:
http://progit.org/ebook/progit.pdf
In the pdf version, Chapter 5 covers "Git on the Server". I was a
little confused at section 5.2, "Getting Git on a Server".
I was expecting a process very similar to installing on a work station
(sections 2.4, "Installing Git", and 2.5, "First-Time Git Setup"),
with differences pertaining to the server. But this section (5.2)
didn't talk about that and seemed to assume I already installed Git on
the server. Instead, this section explains how to create a bare
repository and Section 5.2.1 explains how to put the bare repository
on the server.
So I jumped ahead to section 5.4, "Setting Up the Server", hoping to
find the process I was expecting. For the most part it was there. But
I was left pondering how tedious it would be to manage a couple dozen
git users.
So I read sections 5.7 "Gitosis" and 5.8 "Gitolite". It sounds like
these two tools do the same thing. Since I'm not sure what makes them
different, I wasn't sure which one would fit my needs.
Then, I heard about Gitorious and would like to give that a spin. The
idea of installing a single piece of software that does everything
seems more appealing than installing and configuring multiple pieces
of software.
I'll return to the books for now and do the best I can.
I'm running the Git server in a VirtualBox VM server and keep
snapshots after significant changes. So if I break anything, it will
be a couple clicks to recover.
Thanks for everyone's help.
^ permalink raw reply
* Re: zealous git convert determined to set up git server
From: Luke Diamand @ 2011-09-15 20:59 UTC (permalink / raw)
To: Joshua Stoutenburg; +Cc: Jakub Narebski, Git List
In-Reply-To: <CAOZxsTqtW=DD7zFwQLjknJR8g0nnh0WPUPna6_np4bVoGnSntQ@mail.gmail.com>
On 15/09/11 12:38, Joshua Stoutenburg wrote:
> Breaking away from previous conversation "Anybody home?"
>
> I totally didn't see "The Git Community Book". There's no link for it
> where I was looking: http://git-scm.com/documentation
>
> As for setting up a work station, I found a pretty good guide at GitHub:
> Windows: http://help.github.com/win-set-up-git/
> Linux: http://help.github.com/linux-set-up-git/
>
> Question 1: With both "Pro Git" and "The Git Community Book", has
> anybody noticed any major discrepancies between the pdf and online
> versions? I'd like to place the pdf versions on my mobile device for
> travel reading.
>
> Question 2: It seems gitolite is the popular choice for git user
> management. Any reason why?
I use it. Seems good to me:
- Very easy to install (might just be apt-get install gitolite)
- Git users don't need to have real user accounts on the server machine
- Can do quite sophisticated things with permissions
>
> Question 3: So, Gitorious is more than just a repository hosting
> website? It's also an open source repository hosting platform, which
> powers the Gitorious website? That's pretty cool.
> --
> 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
^ permalink raw reply
* [PATCH v3 00/22] Clean up refname checks and normalization
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
This patch series cleans up refname checks and normalization in a
different way than v1 and v2. (Unnormalized refnames are those that
have extra leading slashes or runs of repeated slashes between
components, like "//refs/heads///master".)
As discussed on the mailing list [1], adding consistent support for
unnormalized refnames everywhere was becoming a bottomless pit, so
instead I am taking the opposite approach--only accept normalized
refnames. Support for unnormalized refnames was broken anyway, so it
shouldn't be missed.
There is only one command that is now meant to accept unnormalized
refnames:
$ git check-ref-format --normalize //refs/heads///master
refs/heads/master
$
This command can be used to normalize refnames for use elsewhere.
I have added a few internal checks that refnames have the correct
format; more tests should still be added. But this patch series is
getting quite long already, so I would like to submit it.
During the development of this patch series, I discovered that
fetch_with_import() in remote-helper.c sometimes passes NULL to
read_ref(), and thereby to resolve_ref(). The two commits labeled
"remote: *" fix this in a naive way. But somebody more familiar with
this code should check whether the fix is OK and more specifically
whether this is a symptom of a bigger problem in the remote-helper
code.
I should mention that this cleanup is preparation for my main goal:
storing ref caches hierarchically. But there is still a lot of
tangled up and seemingly redundant code in refs.c, so it might be a
while before I get to the main project.
[1] http://permalink.gmane.org/gmane.comp.version-control.git/181268
Michael Haggerty (22):
t1402: add some more tests
git check-ref-format: add options --allow-onelevel and
--refspec-pattern
Change bad_ref_char() to return a boolean value
Change check_ref_format() to take a flags argument
Refactor check_refname_format()
Do not allow ".lock" at the end of any refname component
Make collapse_slashes() allocate memory for its result
Inline function refname_format_print()
Change check_refname_format() to reject unnormalized refnames
resolve_ref(): explicitly fail if a symlink is not readable
resolve_ref(): use prefixcmp()
resolve_ref(): only follow a symlink that contains a valid,
normalized refname
resolve_ref(): turn buffer into a proper string as soon as possible
resolve_ref(): extract a function get_packed_ref()
resolve_ref(): do not follow incorrectly-formatted symbolic refs
remote: use xstrdup() instead of strdup()
remote: avoid passing NULL to read_ref()
resolve_ref(): verify that the input refname has the right format
resolve_ref(): emit warnings for improperly-formatted references
resolve_ref(): also treat a too-long SHA1 as invalid
resolve_ref(): expand documentation
add_ref(): verify that the refname is formatted correctly
Documentation/git-check-ref-format.txt | 53 ++++++--
builtin/check-ref-format.c | 61 ++++++---
builtin/checkout.c | 2 +-
builtin/fetch-pack.c | 2 +-
builtin/receive-pack.c | 2 +-
builtin/replace.c | 2 +-
builtin/show-ref.c | 2 +-
builtin/tag.c | 4 +-
cache.h | 34 +++++-
connect.c | 2 +-
environment.c | 2 +-
fast-import.c | 7 +-
git_remote_helpers/git/git.py | 2 +-
notes-merge.c | 5 +-
pack-refs.c | 2 +-
refs.c | 222 +++++++++++++++++++-------------
refs.h | 21 +++-
remote.c | 55 ++------
sha1_name.c | 4 +-
t/t1402-check-ref-format.sh | 120 +++++++++++++++--
transport-helper.c | 10 +-
transport.c | 16 +--
walker.c | 2 +-
23 files changed, 408 insertions(+), 224 deletions(-)
--
1.7.6.8.gd2879
^ permalink raw reply
* [PATCH v3 02/22] git check-ref-format: add options --allow-onelevel and --refspec-pattern
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Also add tests of the new options. (Actually, one big reason to add
the new options is to make it easy to test check_ref_format(), though
the options should also be useful to other scripts.)
Interpret the result of check_ref_format() based on which types of
refnames are allowed. However, because check_ref_format() can only
return a single value, one test case is still broken. Specifically,
the case "git check-ref-format --onelevel '*'" incorrectly succeeds
because check_ref_format() returns CHECK_REF_FORMAT_ONELEVEL for this
refname even though the refname is also CHECK_REF_FORMAT_WILDCARD.
The type of check that leads to this failure is used elsewhere in
"real" code and could lead to bugs; it will be fixed over the next few
commits.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
Documentation/git-check-ref-format.txt | 29 +++++++++--
builtin/check-ref-format.c | 56 +++++++++++++++++---
t/t1402-check-ref-format.sh | 88 +++++++++++++++++++++++++++++---
3 files changed, 152 insertions(+), 21 deletions(-)
diff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt
index c9fdf84..dcb8cc3 100644
--- a/Documentation/git-check-ref-format.txt
+++ b/Documentation/git-check-ref-format.txt
@@ -8,8 +8,8 @@ git-check-ref-format - Ensures that a reference name is well formed
SYNOPSIS
--------
[verse]
-'git check-ref-format' <refname>
-'git check-ref-format' --print <refname>
+'git check-ref-format' [--print]
+ [--[no-]allow-onelevel] [--refspec-pattern] <refname>
'git check-ref-format' --branch <branchname-shorthand>
DESCRIPTION
@@ -32,14 +32,18 @@ git imposes the following rules on how references are named:
. They must contain at least one `/`. This enforces the presence of a
category like `heads/`, `tags/` etc. but the actual names are not
- restricted.
+ restricted. If the `--allow-onelevel` option is used, this rule
+ is waived.
. They cannot have two consecutive dots `..` anywhere.
. They cannot have ASCII control characters (i.e. bytes whose
values are lower than \040, or \177 `DEL`), space, tilde `~`,
- caret `{caret}`, colon `:`, question-mark `?`, asterisk `*`,
- or open bracket `[` anywhere.
+ caret `{caret}`, or colon `:` anywhere.
+
+. They cannot have question-mark `?`, asterisk `{asterisk}`, or open
+ bracket `[` anywhere. See the `--refspec-pattern` option below for
+ an exception to this rule.
. They cannot end with a slash `/` nor a dot `.`.
@@ -78,6 +82,21 @@ were on. This option should be used by porcelains to accept this
syntax anywhere a branch name is expected, so they can act as if you
typed the branch name.
+OPTIONS
+-------
+--allow-onelevel::
+--no-allow-onelevel::
+ Controls whether one-level refnames are accepted (i.e.,
+ refnames that do not contain multiple `/`-separated
+ components). The default is `--no-allow-onelevel`.
+
+--refspec-pattern::
+ Interpret <refname> as a reference name pattern for a refspec
+ (as used with remote repositories). If this option is
+ enabled, <refname> is allowed to contain a single `{asterisk}`
+ in place of a one full pathname component (e.g.,
+ `foo/{asterisk}/bar` but not `foo/bar{asterisk}`).
+
EXAMPLES
--------
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index 0723cf2..7295954 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -8,7 +8,7 @@
#include "strbuf.h"
static const char builtin_check_ref_format_usage[] =
-"git check-ref-format [--print] <refname>\n"
+"git check-ref-format [--print] [options] <refname>\n"
" or: git check-ref-format --branch <branchname-shorthand>";
/*
@@ -45,27 +45,65 @@ static int check_ref_format_branch(const char *arg)
return 0;
}
-static int check_ref_format_print(const char *arg)
+static void refname_format_print(const char *arg)
{
char *refname = xmalloc(strlen(arg) + 1);
- if (check_ref_format(arg))
- return 1;
collapse_slashes(refname, arg);
printf("%s\n", refname);
- return 0;
}
+#define REFNAME_ALLOW_ONELEVEL 1
+#define REFNAME_REFSPEC_PATTERN 2
+
int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
{
+ int i;
+ int print = 0;
+ int flags = 0;
+
if (argc == 2 && !strcmp(argv[1], "-h"))
usage(builtin_check_ref_format_usage);
if (argc == 3 && !strcmp(argv[1], "--branch"))
return check_ref_format_branch(argv[2]);
- if (argc == 3 && !strcmp(argv[1], "--print"))
- return check_ref_format_print(argv[2]);
- if (argc != 2)
+
+ for (i = 1; i < argc && argv[i][0] == '-'; i++) {
+ if (!strcmp(argv[i], "--print"))
+ print = 1;
+ else if (!strcmp(argv[i], "--allow-onelevel"))
+ flags |= REFNAME_ALLOW_ONELEVEL;
+ else if (!strcmp(argv[i], "--no-allow-onelevel"))
+ flags &= ~REFNAME_ALLOW_ONELEVEL;
+ else if (!strcmp(argv[i], "--refspec-pattern"))
+ flags |= REFNAME_REFSPEC_PATTERN;
+ else
+ usage(builtin_check_ref_format_usage);
+ }
+ if (! (i == argc - 1))
usage(builtin_check_ref_format_usage);
- return !!check_ref_format(argv[1]);
+
+ switch (check_ref_format(argv[i])) {
+ case CHECK_REF_FORMAT_OK:
+ break;
+ case CHECK_REF_FORMAT_ERROR:
+ return 1;
+ case CHECK_REF_FORMAT_ONELEVEL:
+ if (!(flags & REFNAME_ALLOW_ONELEVEL))
+ return 1;
+ else
+ break;
+ case CHECK_REF_FORMAT_WILDCARD:
+ if (!(flags & REFNAME_REFSPEC_PATTERN))
+ return 1;
+ else
+ break;
+ default:
+ die("internal error: unexpected value from check_ref_format()");
+ }
+
+ if (print)
+ refname_format_print(argv[i]);
+
+ return 0;
}
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index dc43171..f551eef 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -5,25 +5,38 @@ test_description='Test git check-ref-format'
. ./test-lib.sh
valid_ref() {
- test_expect_success "ref name '$1' is valid" \
- "git check-ref-format '$1'"
+ if test "$#" = 1
+ then
+ test_expect_success "ref name '$1' is valid" \
+ "git check-ref-format '$1'"
+ else
+ test_expect_success "ref name '$1' is valid with options $2" \
+ "git check-ref-format $2 '$1'"
+ fi
}
invalid_ref() {
- test_expect_success "ref name '$1' is not valid" \
- "test_must_fail git check-ref-format '$1'"
+ if test "$#" = 1
+ then
+ test_expect_success "ref name '$1' is invalid" \
+ "test_must_fail git check-ref-format '$1'"
+ else
+ test_expect_success "ref name '$1' is invalid with options $2" \
+ "test_must_fail git check-ref-format $2 '$1'"
+ fi
}
invalid_ref ''
invalid_ref '/'
-valid_ref 'heads/foo'
-invalid_ref 'foo'
+invalid_ref '/' --allow-onelevel
valid_ref 'foo/bar/baz'
valid_ref 'refs///heads/foo'
invalid_ref 'heads/foo/'
valid_ref '/heads/foo'
valid_ref '///heads/foo'
-invalid_ref '/foo'
invalid_ref './foo'
+invalid_ref './foo/bar'
+invalid_ref 'foo/./bar'
+invalid_ref 'foo/bar/.'
invalid_ref '.refs/foo'
invalid_ref 'heads/foo..bar'
invalid_ref 'heads/foo?bar'
@@ -38,6 +51,67 @@ invalid_ref 'heads/foo\bar'
invalid_ref "$(printf 'heads/foo\t')"
invalid_ref "$(printf 'heads/foo\177')"
valid_ref "$(printf 'heads/fu\303\237')"
+invalid_ref 'heads/*foo/bar' --refspec-pattern
+invalid_ref 'heads/foo*/bar' --refspec-pattern
+invalid_ref 'heads/f*o/bar' --refspec-pattern
+
+ref='foo'
+invalid_ref "$ref"
+valid_ref "$ref" --allow-onelevel
+invalid_ref "$ref" --refspec-pattern
+valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+
+ref='foo/bar'
+valid_ref "$ref"
+valid_ref "$ref" --allow-onelevel
+valid_ref "$ref" --refspec-pattern
+valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+
+ref='foo/*'
+invalid_ref "$ref"
+invalid_ref "$ref" --allow-onelevel
+valid_ref "$ref" --refspec-pattern
+valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+
+ref='*/foo'
+invalid_ref "$ref"
+invalid_ref "$ref" --allow-onelevel
+valid_ref "$ref" --refspec-pattern
+valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+
+ref='foo/*/bar'
+invalid_ref "$ref"
+invalid_ref "$ref" --allow-onelevel
+valid_ref "$ref" --refspec-pattern
+valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+
+ref='*'
+invalid_ref "$ref"
+
+#invalid_ref "$ref" --allow-onelevel
+test_expect_failure "ref name '$ref' is invalid with options --allow-onelevel" \
+ "test_must_fail git check-ref-format --allow-onelevel '$ref'"
+
+invalid_ref "$ref" --refspec-pattern
+valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+
+ref='foo/*/*'
+invalid_ref "$ref" --refspec-pattern
+invalid_ref "$ref" '--refspec-pattern --allow-onelevel'
+
+ref='*/foo/*'
+invalid_ref "$ref" --refspec-pattern
+invalid_ref "$ref" '--refspec-pattern --allow-onelevel'
+
+ref='*/*/foo'
+invalid_ref "$ref" --refspec-pattern
+invalid_ref "$ref" '--refspec-pattern --allow-onelevel'
+
+ref='/foo'
+invalid_ref "$ref"
+valid_ref "$ref" --allow-onelevel
+invalid_ref "$ref" --refspec-pattern
+valid_ref "$ref" '--refspec-pattern --allow-onelevel'
test_expect_success "check-ref-format --branch @{-1}" '
T=$(git write-tree) &&
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 06/22] Do not allow ".lock" at the end of any refname component
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Allowing any refname component to end with ".lock" is looking for
trouble; for example,
$ git br foo.lock/bar
$ git br foo
fatal: Unable to create '[...]/.git/refs/heads/foo.lock': File exists.
Therefore, do not allow any refname component to end with ".lock".
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
This change was discussed on the mailing list [1]. It is regrettable
that we can't change the name of the lock files to something that
cannot appear in a reference name (like .refname.lock), but such a
change would cause problems if two versions of git are simultaneously
accessing the same repository.
[1] http://thread.gmane.org/gmane.comp.version-control.git/181051/focus=181069
Documentation/git-check-ref-format.txt | 4 +---
refs.c | 4 ++--
t/t1402-check-ref-format.sh | 8 ++++----
3 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt
index dcb8cc3..9114751 100644
--- a/Documentation/git-check-ref-format.txt
+++ b/Documentation/git-check-ref-format.txt
@@ -28,7 +28,7 @@ git imposes the following rules on how references are named:
. They can include slash `/` for hierarchical (directory)
grouping, but no slash-separated component can begin with a
- dot `.`.
+ dot `.` or end with the sequence `.lock`.
. They must contain at least one `/`. This enforces the presence of a
category like `heads/`, `tags/` etc. but the actual names are not
@@ -47,8 +47,6 @@ git imposes the following rules on how references are named:
. They cannot end with a slash `/` nor a dot `.`.
-. They cannot end with the sequence `.lock`.
-
. They cannot contain a sequence `@{`.
. They cannot contain a `\`.
diff --git a/refs.c b/refs.c
index 5259724..5a0bd0f 100644
--- a/refs.c
+++ b/refs.c
@@ -898,6 +898,8 @@ static int check_refname_component(const char *ref)
return -1; /* Component has zero length. */
if (ref[0] == '.')
return -1; /* Component starts with '.'. */
+ if (cp - ref >= 5 && !memcmp(cp - 5, ".lock", 5))
+ return -1; /* Refname ends with ".lock". */
return cp - ref;
}
@@ -931,8 +933,6 @@ int check_refname_format(const char *ref, int flags)
if (ref[component_len - 1] == '.')
return -1; /* Refname ends with '.'. */
- if (component_len >= 5 && !memcmp(&ref[component_len - 5], ".lock", 5))
- return -1; /* Refname ends with ".lock". */
if (!(flags & REFNAME_ALLOW_ONELEVEL) && component_count < 2)
return -1; /* Refname has only one component. */
return 0;
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index 1cad88f..419788f 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -43,8 +43,8 @@ invalid_ref 'heads/foo?bar'
valid_ref 'foo./bar'
invalid_ref 'heads/foo.lock'
invalid_ref 'heads///foo.lock'
-valid_ref 'foo.lock/bar'
-valid_ref 'foo.lock///bar'
+invalid_ref 'foo.lock/bar'
+invalid_ref 'foo.lock///bar'
valid_ref 'heads/foo@bar'
invalid_ref 'heads/v@{ation'
invalid_ref 'heads/foo\bar'
@@ -160,7 +160,7 @@ invalid_ref_normalized 'heads/./foo'
invalid_ref_normalized 'heads\foo'
invalid_ref_normalized 'heads/foo.lock'
invalid_ref_normalized 'heads///foo.lock'
-valid_ref_normalized 'foo.lock/bar' 'foo.lock/bar'
-valid_ref_normalized 'foo.lock///bar' 'foo.lock/bar'
+invalid_ref_normalized 'foo.lock/bar'
+invalid_ref_normalized 'foo.lock///bar'
test_done
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 08/22] Inline function refname_format_print()
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Soon we will make printing independent of collapsing.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
builtin/check-ref-format.c | 16 +++++++---------
1 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index 989ee5c..f5df9aa 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -48,17 +48,12 @@ static int check_ref_format_branch(const char *arg)
return 0;
}
-static void refname_format_print(const char *arg)
-{
- char *refname = collapse_slashes(arg);
- printf("%s\n", refname);
-}
-
int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
{
int i;
int print = 0;
int flags = 0;
+ const char *refname;
if (argc == 2 && !strcmp(argv[1], "-h"))
usage(builtin_check_ref_format_usage);
@@ -81,11 +76,14 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
if (! (i == argc - 1))
usage(builtin_check_ref_format_usage);
- if (check_refname_format(argv[i], flags))
+ refname = argv[i];
+ if (check_refname_format(refname, flags))
return 1;
- if (print)
- refname_format_print(argv[i]);
+ if (print) {
+ refname = collapse_slashes(refname);
+ printf("%s\n", refname);
+ }
return 0;
}
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 09/22] Change check_refname_format() to reject unnormalized refnames
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Since much of the infrastructure does not work correctly with
unnormalized refnames, change check_refname_format() to reject them.
Similarly, change "git check-ref-format" to reject unnormalized
refnames by default. But add an option --normalize, which causes "git
check-ref-format" to normalize the refname before checking its format,
and print the normalized refname. This is exactly the behavior of the
old --print option, which is retained but deprecated.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
Documentation/git-check-ref-format.txt | 26 ++++++++++++++++++--------
builtin/check-ref-format.c | 15 +++++++--------
refs.c | 3 ---
refs.h | 2 +-
t/t1402-check-ref-format.sh | 31 +++++++++++++++++++++++--------
5 files changed, 49 insertions(+), 28 deletions(-)
diff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt
index 9114751..103e7b1 100644
--- a/Documentation/git-check-ref-format.txt
+++ b/Documentation/git-check-ref-format.txt
@@ -8,8 +8,9 @@ git-check-ref-format - Ensures that a reference name is well formed
SYNOPSIS
--------
[verse]
-'git check-ref-format' [--print]
- [--[no-]allow-onelevel] [--refspec-pattern] <refname>
+'git check-ref-format' [--normalize]
+ [--[no-]allow-onelevel] [--refspec-pattern]
+ <refname>
'git check-ref-format' --branch <branchname-shorthand>
DESCRIPTION
@@ -45,7 +46,11 @@ git imposes the following rules on how references are named:
bracket `[` anywhere. See the `--refspec-pattern` option below for
an exception to this rule.
-. They cannot end with a slash `/` nor a dot `.`.
+. They cannot begin or end with a slash `/` or contain multiple
+ consecutive slashes (see the `--normalize` option below for an
+ exception to this rule)
+
+. They cannot end with a dot `.`.
. They cannot contain a sequence `@{`.
@@ -70,10 +75,6 @@ reference name expressions (see linkgit:gitrevisions[7]):
. at-open-brace `@{` is used as a notation to access a reflog entry.
-With the `--print` option, if 'refname' is acceptable, it prints the
-canonicalized name of a hypothetical reference with that name. That is,
-it prints 'refname' with any extra `/` characters removed.
-
With the `--branch` option, it expands the ``previous branch syntax''
`@{-n}`. For example, `@{-1}` is a way to refer the last branch you
were on. This option should be used by porcelains to accept this
@@ -95,6 +96,15 @@ OPTIONS
in place of a one full pathname component (e.g.,
`foo/{asterisk}/bar` but not `foo/bar{asterisk}`).
+--normalize::
+ Normalize 'refname' by removing any leading slash (`/`)
+ characters and collapsing runs of adjacent slashes between
+ name components into a single slash. Iff the normalized
+ refname is valid then print it to standard output and exit
+ with a status of 0. (`--print` is a deprecated way to spell
+ `--normalize`.)
+
+
EXAMPLES
--------
@@ -107,7 +117,7 @@ $ git check-ref-format --branch @{-1}
* Determine the reference name to use for a new branch:
+
------------
-$ ref=$(git check-ref-format --print "refs/heads/$newbranch") ||
+$ ref=$(git check-ref-format --normalize "refs/heads/$newbranch") ||
die "we do not like '$newbranch' as a branch name."
------------
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index f5df9aa..28a7320 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -8,7 +8,7 @@
#include "strbuf.h"
static const char builtin_check_ref_format_usage[] =
-"git check-ref-format [--print] [options] <refname>\n"
+"git check-ref-format [--normalize] [options] <refname>\n"
" or: git check-ref-format --branch <branchname-shorthand>";
/*
@@ -51,7 +51,7 @@ static int check_ref_format_branch(const char *arg)
int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
{
int i;
- int print = 0;
+ int normalize = 0;
int flags = 0;
const char *refname;
@@ -62,8 +62,8 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
return check_ref_format_branch(argv[2]);
for (i = 1; i < argc && argv[i][0] == '-'; i++) {
- if (!strcmp(argv[i], "--print"))
- print = 1;
+ if (!strcmp(argv[i], "--normalize") || !strcmp(argv[i], "--print"))
+ normalize = 1;
else if (!strcmp(argv[i], "--allow-onelevel"))
flags |= REFNAME_ALLOW_ONELEVEL;
else if (!strcmp(argv[i], "--no-allow-onelevel"))
@@ -77,13 +77,12 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
usage(builtin_check_ref_format_usage);
refname = argv[i];
+ if (normalize)
+ refname = collapse_slashes(refname);
if (check_refname_format(refname, flags))
return 1;
-
- if (print) {
- refname = collapse_slashes(refname);
+ if (normalize)
printf("%s\n", refname);
- }
return 0;
}
diff --git a/refs.c b/refs.c
index 5a0bd0f..d2aac24 100644
--- a/refs.c
+++ b/refs.c
@@ -908,9 +908,6 @@ int check_refname_format(const char *ref, int flags)
int component_len, component_count = 0;
while (1) {
- while (*ref == '/')
- ref++; /* tolerate leading and repeated slashes */
-
/* We are at the start of a path component. */
component_len = check_refname_component(ref);
if (component_len < 0) {
diff --git a/refs.h b/refs.h
index 48540c0..b0da5fc 100644
--- a/refs.h
+++ b/refs.h
@@ -106,7 +106,7 @@ extern int for_each_reflog(each_ref_fn, void *);
* REFNAME_ALLOW_ONELEVEL is set in flags, then accept one-level
* reference names. If REFNAME_REFSPEC_PATTERN is set in flags, then
* allow a "*" wildcard character in place of one of the name
- * components.
+ * components. No leading or repeated slashes are accepted.
*/
extern int check_refname_format(const char *ref, int flags);
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index 419788f..710fcca 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -28,11 +28,17 @@ invalid_ref() {
invalid_ref ''
invalid_ref '/'
invalid_ref '/' --allow-onelevel
+invalid_ref '/' --normalize
+invalid_ref '/' '--allow-onelevel --normalize'
valid_ref 'foo/bar/baz'
-valid_ref 'refs///heads/foo'
+valid_ref 'foo/bar/baz' --normalize
+invalid_ref 'refs///heads/foo'
+valid_ref 'refs///heads/foo' --normalize
invalid_ref 'heads/foo/'
-valid_ref '/heads/foo'
-valid_ref '///heads/foo'
+invalid_ref '/heads/foo'
+valid_ref '/heads/foo' --normalize
+invalid_ref '///heads/foo'
+valid_ref '///heads/foo' --normalize
invalid_ref './foo'
invalid_ref './foo/bar'
invalid_ref 'foo/./bar'
@@ -60,12 +66,15 @@ invalid_ref "$ref"
valid_ref "$ref" --allow-onelevel
invalid_ref "$ref" --refspec-pattern
valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+invalid_ref "$ref" --normalize
+valid_ref "$ref" '--allow-onelevel --normalize'
ref='foo/bar'
valid_ref "$ref"
valid_ref "$ref" --allow-onelevel
valid_ref "$ref" --refspec-pattern
valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+valid_ref "$ref" --normalize
ref='foo/*'
invalid_ref "$ref"
@@ -78,6 +87,8 @@ invalid_ref "$ref"
invalid_ref "$ref" --allow-onelevel
valid_ref "$ref" --refspec-pattern
valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+invalid_ref "$ref" --normalize
+valid_ref "$ref" '--refspec-pattern --normalize'
ref='foo/*/bar'
invalid_ref "$ref"
@@ -105,9 +116,13 @@ invalid_ref "$ref" '--refspec-pattern --allow-onelevel'
ref='/foo'
invalid_ref "$ref"
-valid_ref "$ref" --allow-onelevel
+invalid_ref "$ref" --allow-onelevel
invalid_ref "$ref" --refspec-pattern
-valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+invalid_ref "$ref" '--refspec-pattern --allow-onelevel'
+invalid_ref "$ref" --normalize
+valid_ref "$ref" '--allow-onelevel --normalize'
+invalid_ref "$ref" '--refspec-pattern --normalize'
+valid_ref "$ref" '--refspec-pattern --allow-onelevel --normalize'
test_expect_success "check-ref-format --branch @{-1}" '
T=$(git write-tree) &&
@@ -141,12 +156,12 @@ test_expect_success 'check-ref-format --branch from subdir' '
valid_ref_normalized() {
test_expect_success "ref name '$1' simplifies to '$2'" "
- refname=\$(git check-ref-format --print '$1') &&
+ refname=\$(git check-ref-format --normalize '$1') &&
test \"\$refname\" = '$2'"
}
invalid_ref_normalized() {
- test_expect_success "check-ref-format --print rejects '$1'" "
- test_must_fail git check-ref-format --print '$1'"
+ test_expect_success "check-ref-format --normalize rejects '$1'" "
+ test_must_fail git check-ref-format --normalize '$1'"
}
valid_ref_normalized 'heads/foo' 'heads/foo'
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 07/22] Make collapse_slashes() allocate memory for its result
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
This will make upcoming changes a tiny bit easier.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
builtin/check-ref-format.c | 19 ++++++++++---------
1 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index 8f41696..989ee5c 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -12,25 +12,28 @@ static const char builtin_check_ref_format_usage[] =
" or: git check-ref-format --branch <branchname-shorthand>";
/*
- * Remove leading slashes and replace each run of adjacent slashes in
- * src with a single slash, and write the result to dst.
+ * Return a copy of refname but with leading slashes removed and runs
+ * of adjacent slashes replaced with single slashes.
*
* This function is similar to normalize_path_copy(), but stripped down
* to meet check_ref_format's simpler needs.
*/
-static void collapse_slashes(char *dst, const char *src)
+static char *collapse_slashes(const char *refname)
{
+ char *ret = xmalloc(strlen(refname) + 1);
char ch;
char prev = '/';
+ char *cp = ret;
- while ((ch = *src++) != '\0') {
+ while ((ch = *refname++) != '\0') {
if (prev == '/' && ch == prev)
continue;
- *dst++ = ch;
+ *cp++ = ch;
prev = ch;
}
- *dst = '\0';
+ *cp = '\0';
+ return ret;
}
static int check_ref_format_branch(const char *arg)
@@ -47,9 +50,7 @@ static int check_ref_format_branch(const char *arg)
static void refname_format_print(const char *arg)
{
- char *refname = xmalloc(strlen(arg) + 1);
-
- collapse_slashes(refname, arg);
+ char *refname = collapse_slashes(arg);
printf("%s\n", refname);
}
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 05/22] Refactor check_refname_format()
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Among other things, extract a function check_refname_component().
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 95 +++++++++++++++++++++++++++++++++++++---------------------------
1 files changed, 55 insertions(+), 40 deletions(-)
diff --git a/refs.c b/refs.c
index aaa8730..5259724 100644
--- a/refs.c
+++ b/refs.c
@@ -872,55 +872,70 @@ static inline int bad_ref_char(int ch)
return 0;
}
+/*
+ * Try to read one refname component from the front of ref. Return
+ * the length of the component found, or -1 if the component is not
+ * legal.
+ */
+static int check_refname_component(const char *ref)
+{
+ const char *cp;
+ char last = '\0';
+
+ for (cp = ref; ; cp++) {
+ char ch = *cp;
+ if (ch == '\0' || ch == '/')
+ break;
+ if (bad_ref_char(ch))
+ return -1; /* Illegal character in refname. */
+ if (last == '.' && ch == '.')
+ return -1; /* Refname contains "..". */
+ if (last == '@' && ch == '{')
+ return -1; /* Refname contains "@{". */
+ last = ch;
+ }
+ if (cp == ref)
+ return -1; /* Component has zero length. */
+ if (ref[0] == '.')
+ return -1; /* Component starts with '.'. */
+ return cp - ref;
+}
+
int check_refname_format(const char *ref, int flags)
{
- int ch, level, last;
- const char *cp = ref;
+ int component_len, component_count = 0;
- level = 0;
while (1) {
- while ((ch = *cp++) == '/')
- ; /* tolerate duplicated slashes */
- if (!ch)
- /* should not end with slashes */
- return -1;
-
- /* we are at the beginning of the path component */
- if (ch == '.')
- return -1;
- if (bad_ref_char(ch)) {
- if ((flags & REFNAME_REFSPEC_PATTERN) && ch == '*' &&
- (!*cp || *cp == '/'))
+ while (*ref == '/')
+ ref++; /* tolerate leading and repeated slashes */
+
+ /* We are at the start of a path component. */
+ component_len = check_refname_component(ref);
+ if (component_len < 0) {
+ if ((flags & REFNAME_REFSPEC_PATTERN) &&
+ ref[0] == '*' &&
+ (ref[1] == '\0' || ref[1] == '/')) {
/* Accept one wildcard as a full refname component. */
flags &= ~REFNAME_REFSPEC_PATTERN;
- else
- return -1;
- }
-
- last = ch;
- /* scan the rest of the path component */
- while ((ch = *cp++) != 0) {
- if (bad_ref_char(ch))
- return -1;
- if (ch == '/')
- break;
- if (last == '.' && ch == '.')
- return -1;
- if (last == '@' && ch == '{')
- return -1;
- last = ch;
- }
- level++;
- if (!ch) {
- if (ref <= cp - 2 && cp[-2] == '.')
- return -1;
- if (level < 2 && !(flags & REFNAME_ALLOW_ONELEVEL))
- return -1;
- if (has_extension(ref, ".lock"))
+ component_len = 1;
+ } else {
return -1;
- return 0;
+ }
}
+ component_count++;
+ if (ref[component_len] == '\0')
+ break;
+ /* Skip to next component. */
+ ref += component_len + 1;
}
+
+ if (ref[component_len - 1] == '.')
+ return -1; /* Refname ends with '.'. */
+ if (component_len >= 5 && !memcmp(&ref[component_len - 5], ".lock", 5))
+ return -1; /* Refname ends with ".lock". */
+ if (!(flags & REFNAME_ALLOW_ONELEVEL) && component_count < 2)
+ return -1; /* Refname has only one component. */
+ return 0;
}
const char *prettify_refname(const char *name)
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 01/22] t1402: add some more tests
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
The new tests reflect the status quo. Soon the rule for "*.lock" in
refname components will be tightened up.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
t/t1402-check-ref-format.sh | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index ed4275a..dc43171 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -13,6 +13,8 @@ invalid_ref() {
"test_must_fail git check-ref-format '$1'"
}
+invalid_ref ''
+invalid_ref '/'
valid_ref 'heads/foo'
invalid_ref 'foo'
valid_ref 'foo/bar/baz'
@@ -27,6 +29,9 @@ invalid_ref 'heads/foo..bar'
invalid_ref 'heads/foo?bar'
valid_ref 'foo./bar'
invalid_ref 'heads/foo.lock'
+invalid_ref 'heads///foo.lock'
+valid_ref 'foo.lock/bar'
+valid_ref 'foo.lock///bar'
valid_ref 'heads/foo@bar'
invalid_ref 'heads/v@{ation'
invalid_ref 'heads/foo\bar'
@@ -83,5 +88,9 @@ invalid_ref_normalized '/foo'
invalid_ref_normalized 'heads/foo/../bar'
invalid_ref_normalized 'heads/./foo'
invalid_ref_normalized 'heads\foo'
+invalid_ref_normalized 'heads/foo.lock'
+invalid_ref_normalized 'heads///foo.lock'
+valid_ref_normalized 'foo.lock/bar' 'foo.lock/bar'
+valid_ref_normalized 'foo.lock///bar' 'foo.lock/bar'
test_done
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 04/22] Change check_ref_format() to take a flags argument
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Change check_ref_format() to take a flags argument that indicates what
is acceptable in the reference name (analogous to "git
check-ref-format"'s "--allow-onelevel" and "--refspec-pattern"). This
is more convenient for callers and also fixes a failure in the test
suite (and likely elsewhere in the code) by enabling "onelevel" and
"refspec-pattern" to be allowed independently of each other.
Also rename check_ref_format() to check_refname_format() to make it
obvious that it deals with refnames rather than references themselves.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
builtin/check-ref-format.c | 21 +---------------
builtin/checkout.c | 2 +-
builtin/fetch-pack.c | 2 +-
builtin/receive-pack.c | 2 +-
builtin/replace.c | 2 +-
builtin/show-ref.c | 2 +-
builtin/tag.c | 4 +-
connect.c | 2 +-
environment.c | 2 +-
fast-import.c | 7 +----
git_remote_helpers/git/git.py | 2 +-
notes-merge.c | 5 ++-
pack-refs.c | 2 +-
refs.c | 42 ++++++++++++++-----------------
refs.h | 17 +++++++++----
remote.c | 53 +++++++++++------------------------------
sha1_name.c | 4 +-
t/t1402-check-ref-format.sh | 6 +----
transport.c | 16 +++---------
walker.c | 2 +-
20 files changed, 69 insertions(+), 126 deletions(-)
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index 7295954..8f41696 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -53,9 +53,6 @@ static void refname_format_print(const char *arg)
printf("%s\n", refname);
}
-#define REFNAME_ALLOW_ONELEVEL 1
-#define REFNAME_REFSPEC_PATTERN 2
-
int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
{
int i;
@@ -83,24 +80,8 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
if (! (i == argc - 1))
usage(builtin_check_ref_format_usage);
- switch (check_ref_format(argv[i])) {
- case CHECK_REF_FORMAT_OK:
- break;
- case CHECK_REF_FORMAT_ERROR:
+ if (check_refname_format(argv[i], flags))
return 1;
- case CHECK_REF_FORMAT_ONELEVEL:
- if (!(flags & REFNAME_ALLOW_ONELEVEL))
- return 1;
- else
- break;
- case CHECK_REF_FORMAT_WILDCARD:
- if (!(flags & REFNAME_REFSPEC_PATTERN))
- return 1;
- else
- break;
- default:
- die("internal error: unexpected value from check_ref_format()");
- }
if (print)
refname_format_print(argv[i]);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 3bb6525..574d2b6 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -882,7 +882,7 @@ static int parse_branchname_arg(int argc, const char **argv,
new->name = arg;
setup_branch_path(new);
- if (check_ref_format(new->path) == CHECK_REF_FORMAT_OK &&
+ if (!check_refname_format(new->path, 0) &&
resolve_ref(new->path, branch_rev, 1, NULL))
hashcpy(rev, branch_rev);
else
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 412bd32..b51e478 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -544,7 +544,7 @@ static void filter_refs(struct ref **refs, int nr_match, char **match)
for (ref = *refs; ref; ref = next) {
next = ref->next;
if (!memcmp(ref->name, "refs/", 5) &&
- check_ref_format(ref->name + 5))
+ check_refname_format(ref->name + 5, 0))
; /* trash */
else if (args.fetch_all &&
(!args.depth || prefixcmp(ref->name, "refs/tags/") )) {
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index ae164da..0600efa 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -356,7 +356,7 @@ static const char *update(struct command *cmd)
struct ref_lock *lock;
/* only refs/... are allowed */
- if (prefixcmp(name, "refs/") || check_ref_format(name + 5)) {
+ if (prefixcmp(name, "refs/") || check_refname_format(name + 5, 0)) {
rp_error("refusing to create funny ref '%s' remotely", name);
return "funny refname";
}
diff --git a/builtin/replace.c b/builtin/replace.c
index fe3a647..517fa10 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -94,7 +94,7 @@ static int replace_object(const char *object_ref, const char *replace_ref,
"refs/replace/%s",
sha1_to_hex(object)) > sizeof(ref) - 1)
die("replace ref name too long: %.*s...", 50, ref);
- if (check_ref_format(ref))
+ if (check_refname_format(ref, 0))
die("'%s' is not a valid ref name.", ref);
if (!resolve_ref(ref, prev, 1, NULL))
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 45f0340..fafb6dd 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -145,7 +145,7 @@ static int exclude_existing(const char *match)
if (strncmp(ref, match, matchlen))
continue;
}
- if (check_ref_format(ref)) {
+ if (check_refname_format(ref, 0)) {
warning("ref '%s' ignored", ref);
continue;
}
diff --git a/builtin/tag.c b/builtin/tag.c
index 667515e..48be745 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -407,12 +407,12 @@ static int parse_msg_arg(const struct option *opt, const char *arg, int unset)
static int strbuf_check_tag_ref(struct strbuf *sb, const char *name)
{
if (name[0] == '-')
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
strbuf_reset(sb);
strbuf_addf(sb, "refs/tags/%s", name);
- return check_ref_format(sb->buf);
+ return check_refname_format(sb->buf, 0);
}
int cmd_tag(int argc, const char **argv, const char *prefix)
diff --git a/connect.c b/connect.c
index ee1d4b4..51990fa 100644
--- a/connect.c
+++ b/connect.c
@@ -22,7 +22,7 @@ static int check_ref(const char *name, int len, unsigned int flags)
len -= 5;
/* REF_NORMAL means that we don't want the magic fake tag refs */
- if ((flags & REF_NORMAL) && check_ref_format(name) < 0)
+ if ((flags & REF_NORMAL) && check_refname_format(name, 0))
return 0;
/* REF_HEADS means that we want regular branch heads */
diff --git a/environment.c b/environment.c
index e96edcf..8174b70 100644
--- a/environment.c
+++ b/environment.c
@@ -106,7 +106,7 @@ static char *expand_namespace(const char *raw_namespace)
if (strcmp((*c)->buf, "/") != 0)
strbuf_addf(&buf, "refs/namespaces/%s", (*c)->buf);
strbuf_list_free(components);
- if (check_ref_format(buf.buf) != CHECK_REF_FORMAT_OK)
+ if (check_refname_format(buf.buf, 0))
die("bad git namespace path \"%s\"", raw_namespace);
strbuf_addch(&buf, '/');
return strbuf_detach(&buf, NULL);
diff --git a/fast-import.c b/fast-import.c
index 742e7da..f9347f5 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -722,13 +722,8 @@ static struct branch *new_branch(const char *name)
if (b)
die("Invalid attempt to create duplicate branch: %s", name);
- switch (check_ref_format(name)) {
- case 0: break; /* its valid */
- case CHECK_REF_FORMAT_ONELEVEL:
- break; /* valid, but too few '/', allow anyway */
- default:
+ if (check_refname_format(name, REFNAME_ALLOW_ONELEVEL))
die("Branch name doesn't conform to GIT standards: %s", name);
- }
b = pool_calloc(1, sizeof(struct branch));
b->name = pool_strdup(name);
diff --git a/git_remote_helpers/git/git.py b/git_remote_helpers/git/git.py
index a383e6c..007a1bf 100644
--- a/git_remote_helpers/git/git.py
+++ b/git_remote_helpers/git/git.py
@@ -54,7 +54,7 @@ def valid_git_ref (ref_name):
# The following is a reimplementation of the git check-ref-format
# command. The rules were derived from the git check-ref-format(1)
# manual page. This code should be replaced by a call to
- # check_ref_format() in the git library, when such is available.
+ # check_refname_format() in the git library, when such is available.
if ref_name.endswith('/') or \
ref_name.startswith('.') or \
ref_name.count('/.') or \
diff --git a/notes-merge.c b/notes-merge.c
index e1aaf43..3bbcc9d 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -570,7 +570,8 @@ int notes_merge(struct notes_merge_options *o,
/* Dereference o->local_ref into local_sha1 */
if (!resolve_ref(o->local_ref, local_sha1, 0, NULL))
die("Failed to resolve local notes ref '%s'", o->local_ref);
- else if (!check_ref_format(o->local_ref) && is_null_sha1(local_sha1))
+ else if (!check_refname_format(o->local_ref, 0) &&
+ is_null_sha1(local_sha1))
local = NULL; /* local_sha1 == null_sha1 indicates unborn ref */
else if (!(local = lookup_commit_reference(local_sha1)))
die("Could not parse local commit %s (%s)",
@@ -583,7 +584,7 @@ int notes_merge(struct notes_merge_options *o,
* Failed to get remote_sha1. If o->remote_ref looks like an
* unborn ref, perform the merge using an empty notes tree.
*/
- if (!check_ref_format(o->remote_ref)) {
+ if (!check_refname_format(o->remote_ref, 0)) {
hashclr(remote_sha1);
remote = NULL;
} else {
diff --git a/pack-refs.c b/pack-refs.c
index 1290570..23bbd00 100644
--- a/pack-refs.c
+++ b/pack-refs.c
@@ -72,7 +72,7 @@ static void try_remove_empty_parents(char *name)
for (i = 0; i < 2; i++) { /* refs/{heads,tags,...}/ */
while (*p && *p != '/')
p++;
- /* tolerate duplicate slashes; see check_ref_format() */
+ /* tolerate duplicate slashes; see check_refname_format() */
while (*p == '/')
p++;
}
diff --git a/refs.c b/refs.c
index fd29d89..aaa8730 100644
--- a/refs.c
+++ b/refs.c
@@ -872,10 +872,9 @@ static inline int bad_ref_char(int ch)
return 0;
}
-int check_ref_format(const char *ref)
+int check_refname_format(const char *ref, int flags)
{
int ch, level, last;
- int ret = CHECK_REF_FORMAT_OK;
const char *cp = ref;
level = 0;
@@ -884,41 +883,42 @@ int check_ref_format(const char *ref)
; /* tolerate duplicated slashes */
if (!ch)
/* should not end with slashes */
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
/* we are at the beginning of the path component */
if (ch == '.')
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
if (bad_ref_char(ch)) {
- if (ch == '*' && (!*cp || *cp == '/') &&
- ret == CHECK_REF_FORMAT_OK)
- ret = CHECK_REF_FORMAT_WILDCARD;
+ if ((flags & REFNAME_REFSPEC_PATTERN) && ch == '*' &&
+ (!*cp || *cp == '/'))
+ /* Accept one wildcard as a full refname component. */
+ flags &= ~REFNAME_REFSPEC_PATTERN;
else
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
}
last = ch;
/* scan the rest of the path component */
while ((ch = *cp++) != 0) {
if (bad_ref_char(ch))
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
if (ch == '/')
break;
if (last == '.' && ch == '.')
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
if (last == '@' && ch == '{')
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
last = ch;
}
level++;
if (!ch) {
if (ref <= cp - 2 && cp[-2] == '.')
- return CHECK_REF_FORMAT_ERROR;
- if (level < 2)
- return CHECK_REF_FORMAT_ONELEVEL;
+ return -1;
+ if (level < 2 && !(flags & REFNAME_ALLOW_ONELEVEL))
+ return -1;
if (has_extension(ref, ".lock"))
- return CHECK_REF_FORMAT_ERROR;
- return ret;
+ return -1;
+ return 0;
}
}
}
@@ -1103,7 +1103,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char
struct ref_lock *lock_ref_sha1(const char *ref, const unsigned char *old_sha1)
{
char refpath[PATH_MAX];
- if (check_ref_format(ref))
+ if (check_refname_format(ref, 0))
return NULL;
strcpy(refpath, mkpath("refs/%s", ref));
return lock_ref_sha1_basic(refpath, old_sha1, 0, NULL);
@@ -1111,13 +1111,9 @@ struct ref_lock *lock_ref_sha1(const char *ref, const unsigned char *old_sha1)
struct ref_lock *lock_any_ref_for_update(const char *ref, const unsigned char *old_sha1, int flags)
{
- switch (check_ref_format(ref)) {
- default:
+ if (check_refname_format(ref, REFNAME_ALLOW_ONELEVEL))
return NULL;
- case 0:
- case CHECK_REF_FORMAT_ONELEVEL:
- return lock_ref_sha1_basic(ref, old_sha1, flags, NULL);
- }
+ return lock_ref_sha1_basic(ref, old_sha1, flags, NULL);
}
static struct lock_file packlock;
diff --git a/refs.h b/refs.h
index dfb086e..48540c0 100644
--- a/refs.h
+++ b/refs.h
@@ -97,11 +97,18 @@ int for_each_recent_reflog_ent(const char *ref, each_reflog_ent_fn fn, long, voi
*/
extern int for_each_reflog(each_ref_fn, void *);
-#define CHECK_REF_FORMAT_OK 0
-#define CHECK_REF_FORMAT_ERROR (-1)
-#define CHECK_REF_FORMAT_ONELEVEL (-2)
-#define CHECK_REF_FORMAT_WILDCARD (-3)
-extern int check_ref_format(const char *target);
+#define REFNAME_ALLOW_ONELEVEL 1
+#define REFNAME_REFSPEC_PATTERN 2
+
+/*
+ * Return 0 iff ref has the correct format for a refname according to
+ * the rules described in Documentation/git-check-ref-format.txt. If
+ * REFNAME_ALLOW_ONELEVEL is set in flags, then accept one-level
+ * reference names. If REFNAME_REFSPEC_PATTERN is set in flags, then
+ * allow a "*" wildcard character in place of one of the name
+ * components.
+ */
+extern int check_refname_format(const char *ref, int flags);
extern const char *prettify_refname(const char *refname);
extern char *shorten_unambiguous_ref(const char *ref, int strict);
diff --git a/remote.c b/remote.c
index b8ecfa5..6fcf809 100644
--- a/remote.c
+++ b/remote.c
@@ -493,23 +493,6 @@ static void read_config(void)
}
/*
- * We need to make sure the remote-tracking branches are well formed, but a
- * wildcard refspec in "struct refspec" must have a trailing slash. We
- * temporarily drop the trailing '/' while calling check_ref_format(),
- * and put it back. The caller knows that a CHECK_REF_FORMAT_ONELEVEL
- * error return is Ok for a wildcard refspec.
- */
-static int verify_refname(char *name, int is_glob)
-{
- int result;
-
- result = check_ref_format(name);
- if (is_glob && result == CHECK_REF_FORMAT_WILDCARD)
- result = CHECK_REF_FORMAT_OK;
- return result;
-}
-
-/*
* This function frees a refspec array.
* Warning: code paths should be checked to ensure that the src
* and dst pointers are always freeable pointers as well
@@ -532,13 +515,13 @@ static void free_refspecs(struct refspec *refspec, int nr_refspec)
static struct refspec *parse_refspec_internal(int nr_refspec, const char **refspec, int fetch, int verify)
{
int i;
- int st;
struct refspec *rs = xcalloc(sizeof(*rs), nr_refspec);
for (i = 0; i < nr_refspec; i++) {
size_t llen;
int is_glob;
const char *lhs, *rhs;
+ int flags;
is_glob = 0;
@@ -576,6 +559,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
rs[i].pattern = is_glob;
rs[i].src = xstrndup(lhs, llen);
+ flags = REFNAME_ALLOW_ONELEVEL | (is_glob ? REFNAME_REFSPEC_PATTERN : 0);
if (fetch) {
/*
@@ -585,26 +569,20 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
*/
if (!*rs[i].src)
; /* empty is ok */
- else {
- st = verify_refname(rs[i].src, is_glob);
- if (st && st != CHECK_REF_FORMAT_ONELEVEL)
- goto invalid;
- }
+ else if (check_refname_format(rs[i].src, flags))
+ goto invalid;
/*
* RHS
* - missing is ok, and is same as empty.
* - empty is ok; it means not to store.
* - otherwise it must be a valid looking ref.
*/
- if (!rs[i].dst) {
+ if (!rs[i].dst)
; /* ok */
- } else if (!*rs[i].dst) {
+ else if (!*rs[i].dst)
; /* ok */
- } else {
- st = verify_refname(rs[i].dst, is_glob);
- if (st && st != CHECK_REF_FORMAT_ONELEVEL)
- goto invalid;
- }
+ else if (check_refname_format(rs[i].dst, flags))
+ goto invalid;
} else {
/*
* LHS
@@ -616,8 +594,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
if (!*rs[i].src)
; /* empty is ok */
else if (is_glob) {
- st = verify_refname(rs[i].src, is_glob);
- if (st && st != CHECK_REF_FORMAT_ONELEVEL)
+ if (check_refname_format(rs[i].src, flags))
goto invalid;
}
else
@@ -630,14 +607,12 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
* - otherwise it must be a valid looking ref.
*/
if (!rs[i].dst) {
- st = verify_refname(rs[i].src, is_glob);
- if (st && st != CHECK_REF_FORMAT_ONELEVEL)
+ if (check_refname_format(rs[i].src, flags))
goto invalid;
} else if (!*rs[i].dst) {
goto invalid;
} else {
- st = verify_refname(rs[i].dst, is_glob);
- if (st && st != CHECK_REF_FORMAT_ONELEVEL)
+ if (check_refname_format(rs[i].dst, flags))
goto invalid;
}
}
@@ -1427,8 +1402,8 @@ int get_fetch_map(const struct ref *remote_refs,
for (rmp = &ref_map; *rmp; ) {
if ((*rmp)->peer_ref) {
- int st = check_ref_format((*rmp)->peer_ref->name + 5);
- if (st && st != CHECK_REF_FORMAT_ONELEVEL) {
+ if (check_refname_format((*rmp)->peer_ref->name + 5,
+ REFNAME_ALLOW_ONELEVEL)) {
struct ref *ignore = *rmp;
error("* Ignoring funny ref '%s' locally",
(*rmp)->peer_ref->name);
@@ -1620,7 +1595,7 @@ static int one_local_ref(const char *refname, const unsigned char *sha1, int fla
int len;
/* we already know it starts with refs/ to get here */
- if (check_ref_format(refname + 5))
+ if (check_refname_format(refname + 5, 0))
return 0;
len = strlen(refname) + 1;
diff --git a/sha1_name.c b/sha1_name.c
index ff5992a..143fd97 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -972,9 +972,9 @@ int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
{
strbuf_branchname(sb, name);
if (name[0] == '-')
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
strbuf_splice(sb, 0, 0, "refs/heads/", 11);
- return check_ref_format(sb->buf);
+ return check_refname_format(sb->buf, 0);
}
/*
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index f551eef..1cad88f 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -87,11 +87,7 @@ valid_ref "$ref" '--refspec-pattern --allow-onelevel'
ref='*'
invalid_ref "$ref"
-
-#invalid_ref "$ref" --allow-onelevel
-test_expect_failure "ref name '$ref' is invalid with options --allow-onelevel" \
- "test_must_fail git check-ref-format --allow-onelevel '$ref'"
-
+invalid_ref "$ref" --allow-onelevel
invalid_ref "$ref" --refspec-pattern
valid_ref "$ref" '--refspec-pattern --allow-onelevel'
diff --git a/transport.c b/transport.c
index fa279d5..feb2ff5 100644
--- a/transport.c
+++ b/transport.c
@@ -754,18 +754,10 @@ void transport_verify_remote_names(int nr_heads, const char **heads)
continue;
remote = remote ? (remote + 1) : local;
- switch (check_ref_format(remote)) {
- case 0: /* ok */
- case CHECK_REF_FORMAT_ONELEVEL:
- /* ok but a single level -- that is fine for
- * a match pattern.
- */
- case CHECK_REF_FORMAT_WILDCARD:
- /* ok but ends with a pattern-match character */
- continue;
- }
- die("remote part of refspec is not a valid name in %s",
- heads[i]);
+ if (check_refname_format(remote,
+ REFNAME_ALLOW_ONELEVEL|REFNAME_REFSPEC_PATTERN))
+ die("remote part of refspec is not a valid name in %s",
+ heads[i]);
}
}
diff --git a/walker.c b/walker.c
index dce7128..be389dc 100644
--- a/walker.c
+++ b/walker.c
@@ -190,7 +190,7 @@ static int interpret_target(struct walker *walker, char *target, unsigned char *
{
if (!get_sha1_hex(target, sha1))
return 0;
- if (!check_ref_format(target)) {
+ if (!check_refname_format(target, 0)) {
struct ref *ref = alloc_ref(target);
if (!walker->fetch_ref(walker, ref)) {
hashcpy(sha1, ref->old_sha1);
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 03/22] Change bad_ref_char() to return a boolean value
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Previously most bad characters were indicated by returning 1, but "*"
was special-cased to return 2 instead of 1. One caller examined the
return value to see whether the special case occurred.
But it is easier (to document and understand) for bad_ref_char()
simply to return a boolean value, treating "*" like any other bad
character. Special-case the handling of "*" (which only occurs in
very specific circumstances) at the caller. The resulting calling
code thereby also becomes more transparent.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 15 ++++++---------
1 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c
index a615043..fd29d89 100644
--- a/refs.c
+++ b/refs.c
@@ -860,22 +860,21 @@ int for_each_rawref(each_ref_fn fn, void *cb_data)
* - it contains a "\" (backslash)
*/
+/* Return true iff ch is not allowed in reference names. */
static inline int bad_ref_char(int ch)
{
if (((unsigned) ch) <= ' ' || ch == 0x7f ||
ch == '~' || ch == '^' || ch == ':' || ch == '\\')
return 1;
/* 2.13 Pattern Matching Notation */
- if (ch == '?' || ch == '[') /* Unsupported */
+ if (ch == '*' || ch == '?' || ch == '[') /* Unsupported */
return 1;
- if (ch == '*') /* Supported at the end */
- return 2;
return 0;
}
int check_ref_format(const char *ref)
{
- int ch, level, bad_type, last;
+ int ch, level, last;
int ret = CHECK_REF_FORMAT_OK;
const char *cp = ref;
@@ -890,9 +889,8 @@ int check_ref_format(const char *ref)
/* we are at the beginning of the path component */
if (ch == '.')
return CHECK_REF_FORMAT_ERROR;
- bad_type = bad_ref_char(ch);
- if (bad_type) {
- if (bad_type == 2 && (!*cp || *cp == '/') &&
+ if (bad_ref_char(ch)) {
+ if (ch == '*' && (!*cp || *cp == '/') &&
ret == CHECK_REF_FORMAT_OK)
ret = CHECK_REF_FORMAT_WILDCARD;
else
@@ -902,8 +900,7 @@ int check_ref_format(const char *ref)
last = ch;
/* scan the rest of the path component */
while ((ch = *cp++) != 0) {
- bad_type = bad_ref_char(ch);
- if (bad_type)
+ if (bad_ref_char(ch))
return CHECK_REF_FORMAT_ERROR;
if (ch == '/')
break;
--
1.7.6.8.gd2879
^ permalink raw reply related
* Re: vcs-svn and friends
From: Stephen Bash @ 2011-09-15 21:13 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: David Michael Barr, Dmitry Ivankov, git, Junio C Hamano
In-Reply-To: <20110915204815.GA31508@elie>
----- Original Message -----
> From: "Jonathan Nieder" <jrnieder@gmail.com>
> Sent: Thursday, September 15, 2011 4:48:15 PM
> Subject: Re: vcs-svn and friends
>
> Stephen Bash wrote:
>
> > For those of us interested but out of the loop, does this mean you
> > have a working example where I can point it at a SVN repo and see
> > what happens? Having done our SVN to Git conversion last year, I
> > know our repo has a lot of the common SVN screw cases (non-branching
> > copies, partial merges, mis-merges, *lots* of retagging, changes
> > committed to tags, etc.) so if it's relatively easy to setup a test
> > I'm happy to run one.
>
> Thanks. It's very bare-bones at the moment: it just imports each
> revision as a whole tree, with no branch and merge tracking at all.
> So it would be very interesting to get this basic stuff out into the
> wild and then add some code implementing those things for you to break
> on top of it.
Okay, that matches my last known state of the project, guess I just got optimistic ;)
Since last summer I've been meaning to take a crack at svn-filter-root.py, but it keeps sliding off the priority list. If you guys don't beat me to it, maybe I'll eventually get around to it...
Thanks,
Stephen
^ permalink raw reply
* [PATCH v3 22/22] add_ref(): verify that the refname is formatted correctly
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
In add_ref(), verify that the refname is formatted correctly before
adding it to the ref_list. Here we have to allow refname components
that start with ".", since (for example) the remote protocol uses
synthetic reference name ".have". So add a new REFNAME_DOT_COMPONENT
flag that can be passed to check_refname_format() to allow leading
dots.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 18 ++++++++++++++----
refs.h | 6 +++++-
2 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/refs.c b/refs.c
index 096b42c..832a52f 100644
--- a/refs.c
+++ b/refs.c
@@ -56,6 +56,8 @@ static struct ref_list *add_ref(const char *name, const unsigned char *sha1,
entry = xmalloc(sizeof(struct ref_list) + len);
hashcpy(entry->sha1, sha1);
hashclr(entry->peeled);
+ if (check_refname_format(name, REFNAME_ALLOW_ONELEVEL|REFNAME_DOT_COMPONENT))
+ die("Reference has invalid format: '%s'", name);
memcpy(entry->name, name, len);
entry->flag = flag;
entry->next = list;
@@ -900,7 +902,7 @@ static inline int bad_ref_char(int ch)
* the length of the component found, or -1 if the component is not
* legal.
*/
-static int check_refname_component(const char *ref)
+static int check_refname_component(const char *ref, int flags)
{
const char *cp;
char last = '\0';
@@ -919,8 +921,16 @@ static int check_refname_component(const char *ref)
}
if (cp == ref)
return -1; /* Component has zero length. */
- if (ref[0] == '.')
- return -1; /* Component starts with '.'. */
+ if (ref[0] == '.') {
+ if (!(flags & REFNAME_DOT_COMPONENT))
+ return -1; /* Component starts with '.'. */
+ /*
+ * Even if leading dots are allowed, don't allow "."
+ * as a component (".." is prevented by a rule above).
+ */
+ if (ref[1] == '\0')
+ return -1; /* Component equals ".". */
+ }
if (cp - ref >= 5 && !memcmp(cp - 5, ".lock", 5))
return -1; /* Refname ends with ".lock". */
return cp - ref;
@@ -932,7 +942,7 @@ int check_refname_format(const char *ref, int flags)
while (1) {
/* We are at the start of a path component. */
- component_len = check_refname_component(ref);
+ component_len = check_refname_component(ref, flags);
if (component_len < 0) {
if ((flags & REFNAME_REFSPEC_PATTERN) &&
ref[0] == '*' &&
diff --git a/refs.h b/refs.h
index b0da5fc..d5ac133 100644
--- a/refs.h
+++ b/refs.h
@@ -99,6 +99,7 @@ extern int for_each_reflog(each_ref_fn, void *);
#define REFNAME_ALLOW_ONELEVEL 1
#define REFNAME_REFSPEC_PATTERN 2
+#define REFNAME_DOT_COMPONENT 4
/*
* Return 0 iff ref has the correct format for a refname according to
@@ -106,7 +107,10 @@ extern int for_each_reflog(each_ref_fn, void *);
* REFNAME_ALLOW_ONELEVEL is set in flags, then accept one-level
* reference names. If REFNAME_REFSPEC_PATTERN is set in flags, then
* allow a "*" wildcard character in place of one of the name
- * components. No leading or repeated slashes are accepted.
+ * components. No leading or repeated slashes are accepted. If
+ * REFNAME_DOT_COMPONENT is set in flags, then allow refname
+ * components to start with "." (but not a whole component equal to
+ * "." or "..").
*/
extern int check_refname_format(const char *ref, int flags);
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 21/22] resolve_ref(): expand documentation
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Record information about resolve_ref(), hard-won via reverse
engineering, in a comment for future spelunkers.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
cache.h | 34 +++++++++++++++++++++++++++++++++-
refs.c | 12 ------------
2 files changed, 33 insertions(+), 13 deletions(-)
diff --git a/cache.h b/cache.h
index 607c2ea..aea8685 100644
--- a/cache.h
+++ b/cache.h
@@ -822,7 +822,39 @@ static inline int get_sha1_with_context(const char *str, unsigned char *sha1, st
extern int get_sha1_hex(const char *hex, unsigned char *sha1);
extern char *sha1_to_hex(const unsigned char *sha1); /* static buffer result! */
extern int read_ref(const char *filename, unsigned char *sha1);
-extern const char *resolve_ref(const char *path, unsigned char *sha1, int, int *);
+
+/*
+ * Resolve a reference, recursively following symbolic refererences.
+ *
+ * Store the referred-to object's name in sha1 and return the name of
+ * the non-symbolic reference that ultimately pointed at it. The
+ * return value, if not NULL, is a pointer into either a static buffer
+ * or the input ref.
+ *
+ * If the reference cannot be resolved to an object, the behavior
+ * depends on the "reading" argument:
+ *
+ * - If reading is set, return NULL.
+ *
+ * - If reading is not set, clear sha1 and return the name of the last
+ * reference name in the chain, which will either be a non-symbolic
+ * reference or an undefined reference. If this is a prelude to
+ * "writing" to the ref, the return value is the name of the ref
+ * that will actually be created or changed.
+ *
+ * If flag is non-NULL, set the value that it points to the
+ * combination of REF_ISPACKED (if the reference was found among the
+ * packed references) and REF_ISSYMREF (if the initial reference was a
+ * symbolic reference).
+ *
+ * If ref is not a properly-formatted, normalized reference, return
+ * NULL. If more than MAXDEPTH recursive symbolic lookups are needed,
+ * give up and return NULL.
+ *
+ * errno is sometimes set on errors, but not always.
+ */
+extern const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *flag);
+
extern int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref);
extern int dwim_log(const char *str, int len, unsigned char *sha1, char **ref);
extern int interpret_branch_name(const char *str, struct strbuf *);
diff --git a/refs.c b/refs.c
index 0baa500..096b42c 100644
--- a/refs.c
+++ b/refs.c
@@ -482,18 +482,6 @@ static int get_packed_ref(const char *ref, unsigned char *sha1)
return -1;
}
-/*
- * If the "reading" argument is set, this function finds out what _object_
- * the ref points at by "reading" the ref. The ref, if it is not symbolic,
- * has to exist, and if it is symbolic, it has to point at an existing ref,
- * because the "read" goes through the symref to the ref it points at.
- *
- * The access that is not "reading" may often be "writing", but does not
- * have to; it can be merely checking _where it leads to_. If it is a
- * prelude to "writing" to the ref, a write to a symref that points at
- * yet-to-be-born ref will create the real ref pointed by the symref.
- * reading=0 allows the caller to check where such a symref leads to.
- */
const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *flag)
{
int depth = MAXDEPTH;
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 18/22] resolve_ref(): verify that the input refname has the right format
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/refs.c b/refs.c
index b055501..ee3e0cc 100644
--- a/refs.c
+++ b/refs.c
@@ -504,6 +504,9 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
if (flag)
*flag = 0;
+ if (check_refname_format(ref, REFNAME_ALLOW_ONELEVEL))
+ return NULL;
+
for (;;) {
char path[PATH_MAX];
struct stat st;
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 16/22] remote: use xstrdup() instead of strdup()
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
remote.c | 2 +-
transport-helper.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/remote.c b/remote.c
index 6fcf809..e52aa9b 100644
--- a/remote.c
+++ b/remote.c
@@ -815,7 +815,7 @@ char *apply_refspecs(struct refspec *refspecs, int nr_refspec,
refspec->dst, &ret))
return ret;
} else if (!strcmp(refspec->src, name))
- return strdup(refspec->dst);
+ return xstrdup(refspec->dst);
}
return NULL;
}
diff --git a/transport-helper.c b/transport-helper.c
index 4eab844..0713126 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -183,7 +183,7 @@ static struct child_process *get_helper(struct transport *transport)
ALLOC_GROW(refspecs,
refspec_nr + 1,
refspec_alloc);
- refspecs[refspec_nr++] = strdup(capname + strlen("refspec "));
+ refspecs[refspec_nr++] = xstrdup(capname + strlen("refspec "));
} else if (!strcmp(capname, "connect")) {
data->connect = 1;
} else if (!prefixcmp(capname, "export-marks ")) {
@@ -445,7 +445,7 @@ static int fetch_with_import(struct transport *transport,
if (data->refspecs)
private = apply_refspecs(data->refspecs, data->refspec_nr, posn->name);
else
- private = strdup(posn->name);
+ private = xstrdup(posn->name);
read_ref(private, posn->old_sha1);
free(private);
}
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 10/22] resolve_ref(): explicitly fail if a symlink is not readable
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Previously the failure came later, after a few steps in which the
length was treated like the actual length of a string. Even though
the old code gave the same answers, it was somewhat misleading.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/refs.c b/refs.c
index d2aac24..c51fd45 100644
--- a/refs.c
+++ b/refs.c
@@ -518,6 +518,8 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
/* Follow "normalized" - ie "refs/.." symlinks by hand */
if (S_ISLNK(st.st_mode)) {
len = readlink(path, buffer, sizeof(buffer)-1);
+ if (len < 0)
+ return NULL;
if (len >= 5 && !memcmp("refs/", buffer, 5)) {
buffer[len] = 0;
strcpy(ref_buffer, buffer);
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 14/22] resolve_ref(): extract a function get_packed_ref()
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Making it a function and giving it a name makes the code clearer. I
also have a strong suspicion that the function will find other uses in
the future.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 47 ++++++++++++++++++++++++++++++++++-------------
1 files changed, 34 insertions(+), 13 deletions(-)
diff --git a/refs.c b/refs.c
index 79ab0eb..473f7f6 100644
--- a/refs.c
+++ b/refs.c
@@ -466,6 +466,23 @@ int resolve_gitlink_ref(const char *path, const char *refname, unsigned char *re
}
/*
+ * Try to read ref from the packed references. On success, set sha1
+ * and return 0; otherwise, return -1.
+ */
+static int get_packed_ref(const char *ref, unsigned char *sha1)
+{
+ struct ref_list *list = get_packed_refs(NULL);
+ while (list) {
+ if (!strcmp(ref, list->name)) {
+ hashcpy(sha1, list->sha1);
+ return 0;
+ }
+ list = list->next;
+ }
+ return -1;
+}
+
+/*
* If the "reading" argument is set, this function finds out what _object_
* the ref points at by "reading" the ref. The ref, if it is not symbolic,
* has to exist, and if it is symbolic, it has to point at an existing ref,
@@ -497,22 +514,26 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
return NULL;
git_snpath(path, sizeof(path), "%s", ref);
- /* Special case: non-existing file. */
+
if (lstat(path, &st) < 0) {
- struct ref_list *list = get_packed_refs(NULL);
- while (list) {
- if (!strcmp(ref, list->name)) {
- hashcpy(sha1, list->sha1);
- if (flag)
- *flag |= REF_ISPACKED;
- return ref;
- }
- list = list->next;
+ if (errno != ENOENT)
+ return NULL;
+ /*
+ * The loose reference file does not exist;
+ * check for a packed reference.
+ */
+ if (!get_packed_ref(ref, sha1)) {
+ if (flag)
+ *flag |= REF_ISPACKED;
+ return ref;
}
- if (reading || errno != ENOENT)
+ /* The reference is not a packed reference, either. */
+ if (reading) {
return NULL;
- hashclr(sha1);
- return ref;
+ } else {
+ hashclr(sha1);
+ return ref;
+ }
}
/* Follow "normalized" - ie "refs/.." symlinks by hand */
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 12/22] resolve_ref(): only follow a symlink that contains a valid, normalized refname
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/refs.c b/refs.c
index da9737f..8f0b871 100644
--- a/refs.c
+++ b/refs.c
@@ -521,7 +521,8 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
if (len < 0)
return NULL;
buffer[len] = 0;
- if (!prefixcmp(buffer, "refs/")) {
+ if (!prefixcmp(buffer, "refs/") &&
+ !check_refname_format(buffer, 0)) {
strcpy(ref_buffer, buffer);
ref = ref_buffer;
if (flag)
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 11/22] resolve_ref(): use prefixcmp()
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Terminate the link content string one step earlier, allowing
prefixcmp() to be used instead of the less clear memcmp().
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index c51fd45..da9737f 100644
--- a/refs.c
+++ b/refs.c
@@ -520,8 +520,8 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
len = readlink(path, buffer, sizeof(buffer)-1);
if (len < 0)
return NULL;
- if (len >= 5 && !memcmp("refs/", buffer, 5)) {
- buffer[len] = 0;
+ buffer[len] = 0;
+ if (!prefixcmp(buffer, "refs/")) {
strcpy(ref_buffer, buffer);
ref = ref_buffer;
if (flag)
--
1.7.6.8.gd2879
^ permalink raw reply related
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