Skip to content

AccessViolationException in Proxy.git_remote_push #1217

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
golsby opened this issue Oct 26, 2015 · 10 comments
Closed

AccessViolationException in Proxy.git_remote_push #1217

golsby opened this issue Oct 26, 2015 · 10 comments
Labels
Milestone

Comments

@golsby
Copy link
Contributor

golsby commented Oct 26, 2015

Summary
GitPushOptions (CS) signature mismatch with git_push_options (C) causes uninitialized memory and access violation when calling git_remote_push in libgit2 when credentials are passed for accessing a private repo. The custom_headers member of git_push_options points to random garbage after the call is marshalled because the corresponding member is missing from GitPushOptions.

As a side note, GitHub Desktop cannot push to our private repo, but it doesn't report anything useful when it fails. I imagine this is the cause.

To Reproduce:
Using commit 9d17afe (Oct 23) I'm trying to push using the following code. I'm pushing to a private repo on GitHub using username/personal access token pair.

      if (string.IsNullOrWhiteSpace(Branch))
        Branch = "master";

      using (var repo = new Repository(WorkingTree))
      {
        Log.LogMessage("Pushing branch '{0}' from '{1}'", Branch, WorkingTree);
        LibGit2Sharp.PushOptions options = new LibGit2Sharp.PushOptions();
        options.CredentialsProvider = (_url, _user, _cred) => new UsernamePasswordCredentials { Username = Username, Password = Password };
        var branch = repo.Branches[Branch];
        lock (m_lock)
        {
          repo.Network.Push(branch, options);
        }
      }
      return true;

This results in a 'System.AccessViolationException'.

Details
Debugging into this, I see that git_remote_push on line 2450 of remote.c takes git_push_options as a parameter. From remote.h:

/**
 * Controls the behavior of a git_push object.
 */
typedef struct {
    unsigned int version;

    /**
     * If the transport being used to push to the remote requires the creation
     * of a pack file, this controls the number of worker threads used by
     * the packbuilder when creating that pack file to be sent to the remote.
     *
     * If set to 0, the packbuilder will auto-detect the number of threads
     * to create. The default value is 1.
     */
    unsigned int pb_parallelism;

    /**
     * Callbacks to use for this push operation
     */
    git_remote_callbacks callbacks;

    /**
     * Extra headers for this push operation
     */
    git_strarray custom_headers;
} git_push_options;

The corresponding C# type is GitPushOptions
defined as

namespace LibGit2Sharp.Core
{
    [StructLayout(LayoutKind.Sequential)]
    internal class GitPushOptions
    {
        public int Version = 1;
        public int PackbuilderDegreeOfParallelism;
        public GitRemoteCallbacks RemoteCallbacks;
    }
}

Notice that GitPushOptions is missing the custom_headers member.

@golsby
Copy link
Contributor Author

golsby commented Oct 26, 2015

I attempted to fix this by changing GitPushOptions to be:

    [StructLayout(LayoutKind.Sequential)]
    internal class GitPushOptions
    {
        public int Version = 1;
        public int PackbuilderDegreeOfParallelism;
        public GitRemoteCallbacks RemoteCallbacks;
        public GitStrArrayManaged CustomHeaders;
    }

but I get an AccessViolation exception elsewhere.

@nulltoken nulltoken added the Bug label Oct 27, 2015
@nulltoken
Copy link
Member

@golsby Awesome repro case. Thanks!

@golsby
Copy link
Contributor Author

golsby commented Oct 27, 2015

You're welcome. I hope a fix for this will help the GitHub desktop team release a build that can push to private repos!

@nulltoken
Copy link
Member

@golsby I've granted you with push bit to https://github.com/nulltoken/lg2s_issues_1217 (a private repo I've used to troubleshoot this issue).

By applying the following changes on top of vNext, I've been able to get rid from the first AccessViolationException you mentioned. However, I'm unable to hit the second one.

FWIW, I'm running on a Win10/x64

Any idea?

diff --git a/LibGit2Sharp/Core/GitPushOptions.cs b/LibGit2Sharp/Core/GitPushOptions.cs
index d4bbcdc..075a62b 100644
--- a/LibGit2Sharp/Core/GitPushOptions.cs
+++ b/LibGit2Sharp/Core/GitPushOptions.cs
@@ -8,5 +8,6 @@ internal class GitPushOptions
         public int Version = 1;
         public int PackbuilderDegreeOfParallelism;
         public GitRemoteCallbacks RemoteCallbacks;
+        public GitStrArrayManaged CustomHeaders;
     }
 }
diff --git a/LibGit2Sharp.Tests/PushFixture.cs b/LibGit2Sharp.Tests/PushFixture.cs
index 7dde5ef..77e84e6 100644
--- a/LibGit2Sharp.Tests/PushFixture.cs
+++ b/LibGit2Sharp.Tests/PushFixture.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.IO;
 using System.Linq;
 using LibGit2Sharp.Handlers;
@@ -60,6 +61,44 @@ private void AssertPush(Action<IRepository> push)
             }
         }

+
+        [Fact]
+        public void e1217()
+        {
+            var scd = BuildSelfCleaningDirectory();
+
+            string clonedRepoPath = Repository.Clone("https://github.com/nulltoken/lg2s_issues_1217.git", scd.DirectoryPath,
+                new CloneOptions
+                {
+                    CredentialsProvider = dd,
+                });
+
+            PushOptions options = new PushOptions()
+            {
+                CredentialsProvider = dd,
+                OnPushStatusError = ss,
+            };
+
+            using (var repo = new Repository(clonedRepoPath))
+            {
+                var file = Guid.NewGuid().ToString();
+                Touch(repo.Info.WorkingDirectory, file, "tada");
+                repo.Stage(file);
+                repo.Commit("de", Constants.Signature, Constants.Signature);
+                repo.Network.Push(repo.Network.Remotes["origin"], "HEAD", @"refs/heads/master", options);
+            }
+        }
+
+        private Credentials dd(string url, string usernamefromurl, SupportedCredentialTypes types)
+        {
+            return new UsernamePasswordCredentials { Username = "your_name", Password = "your_password" };
+        }
+
+        private void ss(PushStatusError pushstatuserrors)
+        {
+            Trace.WriteLine(pushstatuserrors.Message);
+        }
+
         [Fact]
         public void CanPushABranchTrackingAnUpstreamBranch()
         {

/cc @carlosmn @dahlbyk @ethomson @jamill @shiftkey to which I've also granted push access to the repo

@golsby
Copy link
Contributor Author

golsby commented Oct 27, 2015

I'll take a look at your branch shortly. I'm running on Windows 10/x64, but inside an MSBuild custom task. It appears that MSBuild is running 32-bit, so my use case ends up loading 32-bit libgit2.

@golsby
Copy link
Contributor Author

golsby commented Oct 28, 2015

Gosh, after paying close attention to figure out the first exception, I didn't actually read the second exception: The branch '{0}' ("{1}") that you are trying to push does not track an upstream branch.

Looks like that's just simple user error! Following these instructions results in a successful push to your test repo.

Thanks for the quick attention!

@nulltoken
Copy link
Member

@golsby As you've done all the hard work (figuring our the bug, reporting it and with a proposal for a fix, ...), how would you feel about sending a PR? 😉

golsby added a commit to golsby/libgit2sharp that referenced this issue Oct 28, 2015
Added CustomHeaders member to GitPushOptions to match underlying c struct for marshalling.
Added tests suggested by @nulltoken to PushFixture.cs
golsby added a commit to golsby/libgit2sharp that referenced this issue Nov 12, 2015
Added CustomHeaders member to GitPushOptions to match underlying c struct for marshalling.
Added tests suggested by @nulltoken to PushFixture.cs
golsby added a commit to golsby/libgit2sharp that referenced this issue Nov 12, 2015
Added CustomHeaders member to GitPushOptions to match underlying c struct for marshalling.
Added tests suggested by @nulltoken to PushFixture.cs
golsby added a commit to golsby/libgit2sharp that referenced this issue Nov 12, 2015
Fix libgit2#1217
Matches underlying c struct for marshalling
golsby added a commit to golsby/libgit2sharp that referenced this issue Nov 12, 2015
Fix libgit2#1217
Matches underlying c struct for marshalling
golsby added a commit to golsby/libgit2sharp that referenced this issue Nov 12, 2015
Fix libgit2#1217
Matches underlying c struct for marshalling
@nulltoken nulltoken added this to the v0.22 milestone Nov 12, 2015
@shiftkey
Copy link
Contributor

As a side note, GitHub Desktop cannot push to our private repo, but it doesn't report anything useful when it fails. I imagine this is the cause.

What sort of server is your private repository on? GitHub? Or somewhere else? I'd love to confirm this is fixed for a future update.

EDIT: Apologies, I missed this line.

I'm pushing to a private repo on GitHub using username/personal access token pair.

Let me have a look at what we're doing in there.

@shiftkey
Copy link
Contributor

I did a quick test on my end with private repositories and I couldn't create it.

If you're still seeing this with GitHub Desktop there's a log file located at %LOCALAPPDATA%\GitHub\TheLog.txt which you can attach to the form at https://github.com/support - this will make it's way to my team for us to investigate further.

@golsby
Copy link
Contributor Author

golsby commented Nov 13, 2015

Hi Brendan,

It looks like we can now push to our repo with the latest GitHub Desktop.
Our repo is very large, so it's not terribly responsive, but it does work.

On Thu, Nov 12, 2015 at 3:12 PM Brendan Forster [email protected]
wrote:

I did a quick test on my end with private repositories and I couldn't
create it.

If you're still seeing this with GitHub Desktop there's a log file located
at %LOCALAPPDATA%\GitHub\TheLog.txt which you can attach to the form at
https://github.com/support - this will make it's way to my team for us to
investigate further.


Reply to this email directly or view it on GitHub
#1217 (comment)
.

  • Brian

Brian Gillespie | Rhinoceros Development | McNeel
(206) 634-4592 | [email protected]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants