Zebras in System.Net.NetworkCredential

Ran into an interesting problem recently where some pretty straightforward FTP code was throwing a WebException on GetResponse when using correct server details.

Looking a little further, the error indicated there was an Object reference not set to an instance of an object (NullReferenceException).

If you’re here for the fix, feel free to jump to Workaround.

Diagnosis

Normally a WebException is pretty informative (usually credential or connectivity issues) but in this case I was getting something unusual:

  • Status: UnknownError
  • Message: Object reference not set to an instance of an object.

Well that’s NullReferenceException text, but this was thrown from framework code and there’s no way the .NET Framework is throwing NullReferenceExceptions on relatively straightforward code right?

Think Horses not Zebras (or its development equivalent Select Isn’t Broken) told me there was something wrong with my code.

We worked on a project where a senior engineer was convinced that the select system call was broken on Solaris. No amount of persuasion or logic could change his mind (the fact that every other networking application on the box worked fine was irrelevant). He spent weeks writing workarounds, which, for some odd reason, didn’t seem to fix the problem. When finally forced to sit down and read the documentation on select, he discovered the problem and corrected it in a matter of minutes. We now use the phrase “select is broken” as a gentle reminder whenever one of us starts blaming the system for a fault that is likely to be our own.

Source: The Pragamtic Programmer via Coding Horror

I reduced my code to the simple test-case below and ran it through LINQPad.

NetworkCredential credentials = new NetworkCredential();
credentials.UserName = "user";
credentials.Password = "pass";
FtpWebRequest request = (FtpWebRequest)FtpWebRequest.Create("ftp://localhost");
request.Credentials = credentials;
request.Method = WebRequestMethods.Ftp.ListDirectoryDetails;

// Throws error
using (FtpWebResponse response = (FtpWebResponse)request.GetResponse())
{
}

But I continued to get the WebException, and noticed that sure enough it was wrapping an inner NullReferenceException

Object reference not set to an instance of an object.  
   at System.Net.NetworkCredential.InternalGetDomainUserName()
   at System.Net.FtpControlStream.BuildCommandsList(WebRequest req)
   at System.Net.CommandStream.SubmitRequest(WebRequest request, Boolean async, Boolean readInitalResponseOnConnect)
   at System.Net.FtpWebRequest.TimedSubmitRequestHelper(Boolean async)
   at System.Net.FtpWebRequest.SubmitRequest(Boolean async)

Checking my code for null values revealed no obvious cause and all signs pointed to a framework bug, but surely someone else would have noticed? I did a search and … found I wasn’t alone!

Unfortunately (at the time) there was no clear explanation for what was happening, so it was time to open the hood with my favourite decompiler and check out System.Net.NetworkCredential.InternalGetDomainUserName

internal string InternalGetDomainUserName()
{
  string str = this.InternalGetDomain();
  if (str.Length != 0)
    str = str + "\\";
  return str + this.InternalGetUserName();
}

Hmm, clearly str must be null to cause the exception I was seeing. What didn’t make sense was why framework code was leaving a potentially null parameter unguarded.

Looking deeper, str was set to InternalGetDomain() which simply returned the local m_domain member.

internal string InternalGetDomain()
{
  return this.m_domain;
}

This meant that m_domain must be null, but checking the property setter for Domain (the public interface to m_domain) I discovered it guards against null values (using string.Empty instead).

public string Domain
{
    set
    {
      if (value == null)
        this.m_domain = string.Empty;
      else
        this.m_domain = value;
    }
}

This explained why the framework code wasn’t guarding (it assumed that the setter property would prevent null values!) but it didn’t explain why m_domain was null.

Hmm, given it was impossible for me to set Domain to null (through the public interface), it was time to see where Domain / m_Domain was initialised (I hadn’t changed it from its default after all).

System.Net.NetworkCredential has a few constructors, most of which chain to a constructor that initialises the username, password, and domain.

public NetworkCredential()
{
}

public NetworkCredential(string userName, string password)
  : this(userName, password, string.Empty)
{
}

public NetworkCredential(string userName, SecureString password)
  : this(userName, password, string.Empty)
{
}

public NetworkCredential(string userName, string password, string domain)
{
  this.UserName = userName;
  this.Password = password;
  this.Domain = domain;
}

public NetworkCredential(string userName, SecureString password, string domain)
{
  this.UserName = userName;
  this.SecurePassword = password;
  this.Domain = domain;
}

However, the default (parameterless) constructor does not chain, and sure enough that’s the one we’re using!

Actually, the default constructor does nothing at all, leaving private members such as Domain uninitialised (and therefore null) – a state that we’ve established for Domain at least is otherwise unachievable through the public interface.

This leads to the following weird behaviours:

// Unaffected: Non-default constructor
new NetworkCredential(
"user", "pass"
); // Domain == String.Empty

// Unaffected: Non-default constructor with named parameters
new NetworkCredential(
userName: "user", password: "pass"
); // Domain == String.Empty

// Impacted: Default constructor with object initializer
new NetworkCredential()
{
Username = "user",
Password = "pass"
}; // Domain == null

// Impacted: Default constructor then property assignment
var creds = new NetworkCredential();
creds.Username = "user";
creds.Password = "pass";
// Domain == null 

Interestingly enough, this doesn’t just impact Domain (UserName and Password are also null). For instance, if you were to use the default constructor and not set a Password, a call to credentials.SecurePassword will throw a NullReferenceException.

NetworkCredential credentials = new NetworkCredential();
// Throws NullReferenceException
object password = credentials.SecurePassword; 

Workaround

Our workaround is therefore:

  • Use another constructor (NetworkCredential credentials = new NetworkCredential("user", "pass");)
  • Or, set Domain to a value (credentials.Domain = null; would do)

Reflections

Now to be fair, the MSDN documentation does remark that the default constructor does not initialise properties.

System.Net.NetworkCredential Default Constructor MSDN Remarks

This being the case though, the framework is not excused from throwing unhandled exceptions and should handle the null properties internally rather than make invalid assumptions.

On Microsoft’s part, this could be fixed by either:

  • Chaining the default constructor
  • Or, initialising members in the default constructor
  • Or, having internal methods such as InternalGetDomainUserName checking members are not null before dereferencing

Outcomes

I lodged a Microsoft Connect ticket for this one and the team was able to verify the issue pretty quickly before advising a fix would be planned in a future release.

I also updated the MSDN Forums question I came across with the solution (the OP had fortunately already arrived at the workaround by accident).