NuGet Core proxy support

Apr 28, 2011 at 11:22 PM

Starting a new discussion thread for the integration of the proxy support for the Core product that can be re-used by the Visual Studio extension and the NuGet.exe command line tool.

Developer
Apr 28, 2011 at 11:45 PM

In general we need to review all the code you wrote in package explorer and determine how the pieces should be split up to support the separate clients. Have you taken a look at how you want to approach the issue?

Apr 28, 2011 at 11:55 PM

Well during my implementation of the proxy support what I have discovered in the Package Explorer was that there were different mechanisms of constructing web requests and and then dealing with web responses.
One thing that I would like to do first is to centralize that logic so that we can implement the proxy support very closely with the web request functionality so that we have the code in the same place.
I have also created a NuGet package called CredentialManagement that is going to enable us to use the new style of UI that Windows Vista and above supports.
My next step was to extract the current ProxyService implementation from the Package Explorer and make a new NuGet package out of that that is going to depend on this new CredentialManagement package.
This way others can re-use this functionality in their own tools if they like.
I will now have to start digging through the NuGet core implementation and figure out how the web requests are made and what we might need to refactor there if anything to support what I have described above.
If you have any suggestions/recommendations please let me know and I can start looking into that.

Apr 29, 2011 at 4:03 AM

I just created the first version of the ProxyFinder project on CodeProject that you can find here: http://proxyfinder.codeplex.com/

I still need to finish up some coding for it but that should not take long and once this is done we can take a look at what needs to be updated and how we can integrate it into NuGet Core.
Let me know if I am missing something.

Apr 30, 2011 at 3:53 AM
Edited Apr 30, 2011 at 4:56 AM

@dfowler: Would there be a problem if I try to use the two NuGet packages that I've mentioned above to integrate this proxy functionality into the Core NuGet repository?
I've set the licenses on both of the packages to the same one that NuGet is using and the codebase is actually based on the same work that was done for the Package Explorer except that the CredentialManagement package
is a wrapper re-write in C# that goes against the Windows Credential Management API and allows you to use the new style of the credentials dialog if you are running windows Vista and above.
The question is weather the core dev team thinks that it is OK to use the NuGet packages in the main NuGet project and their related assemblies possibly. This way when I come out with updates to the two libraries
everyone including NuGet is going to be able benefit.

Apr 30, 2011 at 4:58 AM

That's an interesting idea, but I'm afraid referencing two external packages will result int two extra .dlls. We really want to avoid that.

Apr 30, 2011 at 5:27 AM
Edited Apr 30, 2011 at 5:34 AM

I understand the concern. But wouldn't it be better if NuGet was "eating it own dog food"? It would also be really nice to not have to maintain two code bases one that would be an exact copy that would live inside of NuGet and the other that would be as a separate package or packages for people to use. Either way the functionality needs to make it into the NuGet product I am just trying to figure out the best way that the dev team thinks of consuming this functionality.

If you think that the best way to integrate these features is to place the code somewhere into the Core library so that all of the other assemblies can re-use it, would it be a good idea to completely encapsulate the logic of searching for the Proxy directly into the HttpClient and refactor the rest of the code base so that everything is consistently using HttpClient class for creating the WebRequest objects so that we can centrally manage the proxy detection and caching logic? Also where would you envision placing the Credential Management functionality if it is to be placed somewhere in the Core library so that the ProxyService can utilize it?

Suggestions/comments/recommendations are greatly appreciated.

Developer
Apr 30, 2011 at 7:15 AM

Unfortunately we also ship NuGet.Core with other products ASP.NET WebPages (NuGet.Core is in the GAC), Orchard so those are things to consider. We try not to have alot of external dependencies in the Core. I agree with @dotnetjunky prefer it if the code was part of NuGet itself.

Given that I totally agree with your approach everything except for the CredentialDialog should be in core. Each client should be able to decide how it wants to accept credentials. We also need to make sure that NuGet.Core is security transparent (that means no p-invoking native code). 

Apr 30, 2011 at 2:52 PM

I see. I didn't know that this was the case @dfowler. This makes much more sense now and I agree that each client should be capable of doing their own credentials dialog approach if needed especially that web sites don't have a Windows UI.

I will start with refactoring something small then maybe the console app and see if everyone likes that approach and then we can implement it in the rest of the application.
Thanks for the nudge in the right direction.

May 1, 2011 at 3:38 AM

@dfowler & @dotnetjunky: can the both of you take a look at the way that I've updated the Bootstrapper library to use the HttpClient along with the way that I refactored the proxy detection logic.
The changes can be found in the following fork: https://hg01.codeplex.com/forks/ilyalozovyy/commandlineproxy/

I have basically refactored the way that the ProxyFinder was performing checks for a valid proxy and how it was obtaining the proxy and the credentials for each proxy and I believe that this new
pattern should help us to be able to easily move the proxy logic from one library to another without creating many dependencies on each library.
This pattern can also be added as a base implementation to the Core library and then if each individual client decides that they want to either add/update the way that proxies and their credentials
are discovered then all it would take is to create a new class that will derive from BaseProxyFinder and come up with their own IProxyFinderStrategy implementations.
Hopefully I was able to explain my implementation good enough for you to get the basics. Please let me know if you need more information.
Also don't pay too close of an attention to the multiple classes and interfaces that live within a couple of classes that I have added to that library as it was there just for me to be able to easily move things around.

May 3, 2011 at 7:25 PM
Edited May 3, 2011 at 7:27 PM

@ilya: It looks like you have sent out code reviews for this. Fowler and I have reviewed and commented on it.

Now, regarding the integration into NuGet Visual Studio extension, it has been brought to my attention that VS does have a service to help with proxy detection. It's called IVsWebProxy (http://msdn.microsoft.com/en-us/library/microsoft.visualstudio.shell.interop.ivswebproxy.aspx). VS itself uses this service to connect to web resources. The MSDN page doesn't provide any code sample, but luckily Saurabh Jain (he's the one who implemented this interface) has provided one here (http://social.msdn.microsoft.com/Forums/en-US/vsx/thread/de1d3cf1-a9ef-4d93-91c3-11d78415938c/).

So that's good news. We don't have to repeat the detection logics in nuget visual studio, and we don't have to depend on Kerr.dll. And the logics recommended by him is very similar to what you have done in Package Explorer ;-)

Let me know if you have any question. I hope you haven't written lots of code for nuget visual studio.

May 3, 2011 at 7:36 PM

@dotnetjunky: Thanks for the heads up. I have not started implementing the code for Visual Studio yet so this is good news that there is something that is already available. I am hoping to implement the proxy detection logic in the smaller parts of the NuGet such as in the CommandLine tool after I've finished with the Bootstrapper and then move on to other bigger parts of the application. I'll take a look at the code comments later on today and respond accordingly.

May 8, 2011 at 12:46 AM

@dotnetjunky: So after looking at the sample that you linked to I am having some trouble figuring out how it is supposed to work. After running this code in the proxy environment I am not able to get it to force the prompt to come up even thought I am specifying the property correctly and actually on the second attempt of the code to try and to set the proxy settings again since it is in a while loop it throws an exception and the logic aborts. I am curious if you can ask someone on the Visual Studio team to see if they could shed some light on this problem. Other than that I was able to refactor the rest of the NuGet code for all of the pieces on the main solution to start using the new HttpClient implementation with the new ProxyFinder support that I think is pretty easy to consume and easy to extend. It actually follows the WebRequest.DefaultWebProxy pattern.

Let me know how you would like to proceed.

Developer
May 9, 2011 at 3:41 AM

Let's take the changes in pieces. 

  1. Get the core changes in so that other clients can use them.
  2. Do the command line and bootstrapper.
  3. Then VS.

How does that sound?

May 9, 2011 at 7:20 AM

@ilyalozovyy: Let's make the core change first as dfowler suggested. Meanwhile, I'll play with the the sample for VS and give you sample code when I get it to work. Sounds good?

May 9, 2011 at 3:15 PM

Sounds good to me. Item #1 was checked in and a new code review request submitted.

May 11, 2011 at 3:15 PM

Just wanted to give a quick update. After I got the comments in the code review process I ran the End To End tests and realized that I needed to address some issues to say the least. I have made some code updates and I am hoping to resolve the remaining issues shortly and continue with the code review process.

May 12, 2011 at 6:41 PM
ilyalozovyy wrote:

@dotnetjunky: So after looking at the sample that you linked to I am having some trouble figuring out how it is supposed to work. After running this code in the proxy environment I am not able to get it to force the prompt to come up even thought I am specifying the property correctly and actually on the second attempt of the code to try and to set the proxy settings again since it is in a while loop it throws an exception and the logic aborts. I am curious if you can ask someone on the Visual Studio team to see if they could shed some light on this problem. Other than that I was able to refactor the rest of the NuGet code for all of the pieces on the main solution to start using the new HttpClient implementation with the new ProxyFinder support that I think is pretty easy to consume and easy to extend. It actually follows the WebRequest.DefaultWebProxy pattern.

Let me know how you would like to proceed.


Hi Ilya. I checked with the vs team. They said the sample code shows how it is supposed to be used. I tried it myself and it didn't throw any exception, but that may be because my proxy doesn't require authentication. 

Did you copy the code as is or did you make any change to it?

May 12, 2011 at 7:04 PM

Here is the code that I used

using System;
using System.Net;
using Microsoft.VisualStudio;
using Microsoft.VisualStudio.Shell.Interop;

namespace NuGet.VisualStudio {
    public class VisualStudioCredentialStrategy : BaseProxyFinderStrategy {

        /// <summary>
        /// Represents a delegate that executes any method in managed code that is declared void and takes no parameters.
        /// </summary>
        public delegate void VsWebCall();
        /// <summary>
        /// Represents a delegate to do some call specific processing on the proxy authentication exception.
        /// </summary>
        /// <param name="ex">The proxy authentication exception.</param>
        public delegate void VsProxyAuthenticaitonExceptionHandler(WebException ex);

        public override IWebProxy GetProxy(Uri uri) {
            WebProxy systemProxy = GetSystemProxy(uri);
            __VsWebProxyState proxyState = __VsWebProxyState.VsWebProxyState_NoCredentials;
            WebException lastException = null;

            MakeWebCall(
                    uri.AbsoluteUri,
                    ref proxyState,
                    true,
                    delegate {
                        var request = WebRequest.Create(uri.AbsoluteUri);
                        var response = request.GetResponse();
                    },
                    delegate(WebException ex) {
                        lastException = ex;
                    }
                );

            return proxyState == __VsWebProxyState.VsWebProxyState_Abort ? null : systemProxy;
        }

        void MakeWebCall(string url, ref __VsWebProxyState proxyState, bool okToPrompt, VsWebCall vsWebCall, VsProxyAuthenticaitonExceptionHandler proxyAuthenticationExceptionHandler) {
            bool madeWebCall = false;

            uint curState = 0;

            while(true) {
                uint outState = 0;
                IVsWebProxy proxyService = ServiceLocator.GetGlobalService<SVsWebProxy, IVsWebProxy>();
                if(proxyService == null) {
                    //Debug.Fail("Unable to get the web proxy service");
                    // We should still make one Web call even when we are not able to get
                    // the proxy service.
                    if(madeWebCall) {
                        proxyState = __VsWebProxyState.VsWebProxyState_Abort;
                    }
                }
                else {
                    uint state = (uint)proxyState;
                    //int hr = proxyService.PrepareWebProxy(url, state, out state, Convert.ToInt32(okToPrompt));
                    int hr = proxyService.PrepareWebProxy(url, curState, out outState, Convert.ToInt32(okToPrompt));
                    if(hr != VSConstants.S_OK) {
                        throw System.Runtime.InteropServices.Marshal.GetExceptionForHR(hr);
                    }
                    proxyState = (__VsWebProxyState)state;
                    curState++;
                }
                if(proxyState == __VsWebProxyState.VsWebProxyState_Abort) {
                    break;
                }
                try {
                    madeWebCall = true;
                    if(vsWebCall != null) {
                        vsWebCall();
                    }
                }
                catch(WebException ex) {
                    //if (ProxyAuthenticationRequired(ex))
                    var httpResponse = ex.Response as HttpWebResponse;
                    if(httpResponse != null && httpResponse.StatusCode == HttpStatusCode.ProxyAuthenticationRequired) {
                        if(proxyAuthenticationExceptionHandler != null) {
                            proxyAuthenticationExceptionHandler(ex);
                        }
                        // Retry if proxy authentication (error 407) is required.
                        continue;
                    }
                    throw;
                }
                break;
            }
        }
    }
}

May 12, 2011 at 7:18 PM

Here is the definition for ProxyAuthenticationRequired() method:

        public static bool ProxyAuthenticationRequired(Exception ex)
        {
            bool authNeeded = false;

            System.Net.WebException wex = ex as System.Net.WebException;

            if ((wex != null) && (wex.Status == System.Net.WebExceptionStatus.ProtocolError))
            {
                System.Net.HttpWebResponse hwr = wex.Response as System.Net.HttpWebResponse;
                if ((hwr != null) && (hwr.StatusCode == System.Net.HttpStatusCode.ProxyAuthenticationRequired))
                {
                    authNeeded = true;
                }
            }

            return authNeeded;
        }

It has the check for ProtocolError. Not sure if it will make a difference, but worth a try. Also, this method doesn't work with the forward link, AFAIK. You may want to change the package source to: http://packages.nuget.org/v1/FeedService.svc instead of our forward link.

May 12, 2011 at 7:42 PM

I am getting this return code from the PrepareWebProxy: -2146233079
This happens on the second round in the while loop and is the same behavior that I was seeing before.

Here is the new code based on what you've recommended.

using System;
using System.Net;
using Microsoft.VisualStudio;
using Microsoft.VisualStudio.Shell.Interop;

namespace NuGet.VisualStudio {
    public class VisualStudioCredentialStrategy : BaseProxyFinderStrategy {

        /// <summary>
        /// Represents a delegate that executes any method in managed code that is declared void and takes no parameters.
        /// </summary>
        public delegate void VsWebCall();
        /// <summary>
        /// Represents a delegate to do some call specific processing on the proxy authentication exception.
        /// </summary>
        /// <param name="ex">The proxy authentication exception.</param>
        public delegate void VsProxyAuthenticaitonExceptionHandler(WebException ex);

        public override IWebProxy GetProxy(Uri uri) {
            WebProxy systemProxy = GetSystemProxy(uri);
            __VsWebProxyState proxyState = __VsWebProxyState.VsWebProxyState_NoCredentials;
            WebException lastException = null;

            // Hard Coded Uri for testing only.
            uri = new Uri("http://packages.nuget.org/v1/FeedService.svc/");

            MakeWebCall(
                    uri.AbsoluteUri,
                    ref proxyState,
                    true,
                    delegate {
                        var request = WebRequest.Create(uri.AbsoluteUri);
                        var response = request.GetResponse();
                    },
                    delegate(WebException ex) {
                        lastException = ex;
                    }
                );

            return proxyState == __VsWebProxyState.VsWebProxyState_Abort ? null : systemProxy;
        }

        void MakeWebCall(string url, ref __VsWebProxyState proxyState, bool okToPrompt, VsWebCall vsWebCall, VsProxyAuthenticaitonExceptionHandler proxyAuthenticationExceptionHandler) {
            bool madeWebCall = false;

            while(true) {
                IVsWebProxy proxyService = ServiceLocator.GetGlobalService<SVsWebProxy, IVsWebProxy>();
                if(proxyService == null) {
                    // We should still make one Web call even when we are not able to get
                    // the proxy service.
                    if(madeWebCall) {
                        proxyState = __VsWebProxyState.VsWebProxyState_Abort;
                    }
                }
                else {
                    uint state = (uint)proxyState;
                    int hr = proxyService.PrepareWebProxy(url, state, out state, Convert.ToInt32(okToPrompt));
                    if(hr != VSConstants.S_OK) {
                        throw System.Runtime.InteropServices.Marshal.GetExceptionForHR(hr);
                    }
                    proxyState = (__VsWebProxyState)state;
                }
                if(proxyState == __VsWebProxyState.VsWebProxyState_Abort) {
                    break;
                }
                try {
                    madeWebCall = true;
                    if(vsWebCall != null) {
                        vsWebCall();
                    }
                }
                catch(WebException ex) {
                    if(ProxyAuthenticationRequired(ex)) {
                        if(proxyAuthenticationExceptionHandler != null) {
                            proxyAuthenticationExceptionHandler(ex);
                        }
                        // Retry if proxy authentication (error 407) is required.
                        continue;
                    }
                    throw;
                }
                break;
            }
        }

        private static bool ProxyAuthenticationRequired(Exception ex) {
            bool authNeeded = false;

            System.Net.WebException wex = ex as System.Net.WebException;

            if((wex != null) && (wex.Status == System.Net.WebExceptionStatus.ProtocolError)) {
                System.Net.HttpWebResponse hwr = wex.Response as System.Net.HttpWebResponse;
                if((hwr != null) && (hwr.StatusCode == System.Net.HttpStatusCode.ProxyAuthenticationRequired)) {
                    authNeeded = true;
                }
            }

            return authNeeded;
        }
    }
}

May 12, 2011 at 8:19 PM

Interesting. I'm asking the VS guys. Stay tuned.

May 12, 2011 at 11:10 PM

Ilya, from where did you call this method? Which thread is it in? It should be called from UI thread if possible.

May 12, 2011 at 11:20 PM

This is one of those Providers/Strategies that I am using in the new Proxy implementation so that it first tries the No Proxy first, then Integrated Authentication, and then this provider gets called.
However I think that if we can get this method working then I don't think that the VisualStudio client is going to need the other providers since based on the API for PrepareProxy it looks like it should be able to handle all of these scenarios.

May 12, 2011 at 11:27 PM

Yes, the while loop tries through all the states (no proxy, integrated authentication and then prompt). You don't need the other providers.

Can you put a break point in there and see what thread is invoking it? Also, are you calling it from the Add Package dialog or the Console?

May 12, 2011 at 11:54 PM

I've tried both from console and from the dialog. Some times I see the call being executed on the Main Thread and some times on the Worker Thread.
The interesting thing about this PrepareWebProxy method is that based on the name I would assume that it should set up the proxy somewhere, but does it do it for every WebRequest? if so how does it do it?
As the name suggests that it prepares the proxy so is there a way to get that proxy after the call?

May 13, 2011 at 12:04 AM

Does it always fail either on Main thread or worker thread?

Yes, it prepares the proxy for the address so that every subsequent calls to WebRequest.Create() with the same address will automatically have the correct credentails attached. I don't know clearly how it does it. :)

May 13, 2011 at 12:14 AM

Yes, It fails every time no matter on what thread and always with the same error code.

May 13, 2011 at 9:49 PM

At this step, they have no clue. But they told me that the error code correspond to: COR__INVALIDOPERATION error.

May 13, 2011 at 11:05 PM
So do you think that it makes sense to move on and use a custom implementation similar to what we did for Package Explorer until we find a better way?
Also I haven't seen any responses to my comments in the code review and I am wondering if you are waiting for something from me or just haven't had a chance to look at it yet.
Let me know if you want me to update the diff so you can look at the latest version of the code.

On Fri, May 13, 2011 at 4:50 PM, dotnetjunky <notifications@codeplex.com> wrote:

From: dotnetjunky

At this step, they have no clue. But they told me that the error code correspond to: COR__INVALIDOPERATION error.

Read the full discussion online.

To add a post to this discussion, reply to this email (nuget@discussions.codeplex.com)

To start a new discussion for this project, email nuget@discussions.codeplex.com

You are receiving this email because you subscribed to this discussion on CodePlex. You can unsubscribe on CodePlex.com.

Please note: Images and attachments will be removed from emails. Any posts to this discussion will also be available online at CodePlex.com




--
Regards,

Ilya Lozovyy
May 13, 2011 at 11:12 PM

Agreed. Let's work on a custom implementation for now. Can you code it in such a way that we can easily switch back to using IVsWebProxy once we've figure out how to use it? Thanks

May 13, 2011 at 11:23 PM

Yes, the new implementation is modularized specifically for this reason that we can easily switch any implementation of the way that the proxy is detected without having to re-write much of the code.
The only thing I am not sure about is if I should wait until the core proxy implementation has been fully reviewed by the dev team before I continue further development as we've agreed to the 3 step approach and that is why I mentioned the code review in my prior post.
So just let me know how you would like to proceed with it.