Authenticating web request to get the xml data from it

Question

I wrote a method early of our development last year about getting the content of a webrequest where it contains an xml data that we need to process and insert to database. I wanted to improve this, so I'm asking a feedback on these code snippets.

public async Task<BulkStatusCode> AuthenticateConnection(string url, string user, string access, string recordType)
{
    try
    {
        // TODO : this is just a quickfix, create a logic that will determine if the url is using ssl or not
        url = url.Replace("http://", "https://");

        var uri = new Uri(url);
        //ServicePointManager.ServerCertificateValidationCallback = ((sender, certificate, chain, SslPolicyErrors) => true);

        var request = WebRequest.Create(uri) as HttpWebRequest;

        request.PreAuthenticate = true;
        //will ignore certificate validation
        request.ServerCertificateValidationCallback = ((sender, certificate, chain, SslPolicyErrors) => true);
        request.Method = WebRequestMethods.Http.Get;
        request.Credentials = BulkAuthentication.GetCredentialCache(uri, user, access);

        //check if status is 200 and content type is correct
        using (var response = await request.GetResponseAsync() as HttpWebResponse)
        {
            if (response.StatusCode.Equals(HttpStatusCode.OK))
            {
                if (!ReferenceEquals(response.ContentType, null) && IsValidContentType(response.ContentType))
                {
                    using (var stream = response.GetResponseStream())
                    {
                        //store stream to variable
                        using (var ms = new MemoryStream())
                        {
                            stream.CopyTo(ms);
                            var storeStreamToCache = ms.ToArray();
                            var storeStreamToValidate = ms.ToArray();

                            using (Stream streamToValidate = new MemoryStream(storeStreamToValidate))
                            {
                                using (var reader2 = new StreamReader(streamToValidate))
                                {
                                    if (!XmlValidator.IsValidXml(reader2, XmlSchemaType(recordType)))
                                        return BulkStatusCode.UnsupportedXML;
                                }
                            }

                            using (Stream streamToCache = new MemoryStream(storeStreamToCache))
                            {
                                using (var reader = new StreamReader(streamToCache))
                                {
                                    if (recordType.Equals("Foo"))
                                    {
                                        var serializer = new XmlSerializer(typeof(BulkFooRoot));
                                        userXmlData = (BulkFooRoot)serializer.Deserialize(reader);
                                        _cache.Add("BulkFooUserXml", fooXmlData);
                                    }
                                    else if (recordType.Equals("Bar"))
                                    {
                                        var serializer = new XmlSerializer(typeof(BulkBarRoot));
                                        projectXmlData = (BulkBarRoot)serializer.Deserialize(reader);
                                        _cache.Add("BulkUploadBarXml", projectXmlData);
                                    }
                                }
                            }
                        }
                    }
                    //valid content type but has invalid xml content
                    return BulkStatusCode.OK;
                }
                //invalid content type
                return BulkStatusCode.UnsupportedMediaType;
            }
        }
    }
    catch (WebException ex)
    {
        var responseStatusCode = ((HttpWebResponse)ex.Response).StatusCode;
        if (ReferenceEquals(ex.Response, null) || !responseStatusCode.Equals(HttpStatusCode.Unauthorized))
            //invalid request (http not found)
            return BulkStatusCode.NotFound;
    }
    // retry status to retry to login again
    return BulkStatusCode.Retry;
}

private static bool IsValidContentType(string contentType)
{
    switch (contentType)
    {
        case "text/xml":
        case "application/xml; charset=utf-8":
            return true;
    }
    return false;
}

Answer 1

A few notes to kick you off:

// TODO : this is just a quickfix, create a logic that will determine if the url is using ssl or not
url = url.Replace("http://", "https://");
var uri = new Uri(url);

I consider updating input parameters to be a really bad idea... Factor out a method to handle this:

var requestUri = GetSecureUri(url);

which could look something like:

public Uri GetSecureUri(string url)
{
   if (string.IsNullOrEmpty(url))
   {
       throw new ArgumentNullException("url");
   }
   Uri requestUri;
   if (!Uri.TryCreate(url, UriKind.Absolute, out requestUri))
   {
       throw new ArgumentException("url must be a valid absolute URI.", "url");
   }
   if (requestUri.Scheme == Uri.UriSchemeHttps)
   {
       return requestUri;
   }
   return new UriBuilder(requestUri)
   {
       Port = requestUri.IsDefaultPort ? -1 : requestUri.Port,
       Scheme = Uri.UriSchemeHttps

   }.Uri;
}

Is that "better"? Maybe - but probably not worth all the extra effort.

---

//will ignore certificate validation
request.ServerCertificateValidationCallback = ((sender, certificate, chain, SslPolicyErrors) => true);

No! NO! NO! NO! NO! NO!. If you're going to force TLS (a good thing for sensitive information) you need to check the certificate. If you ignore all the validation then someone could give you a self signed cert for a service they don't own and you wouldn't even know. Don't do this. I can't even find words to use to fully convey the amount I've just lost faith in humanity.

---

Use the normal cast when you know it's going to succeed: WebRequest.Create(uri) as HttpWebRequest should be (HttpWebRequest)WebRequest.Create(uri).

I'll probably come back later and review a bit more.

Answer 2

Your nesting is so deep that I need a scuba tank to keep diving ;-)

To reduce it you should return from the method as soon as the condition is false and there is no else rather then nesting another if inside.

if (!response.StatusCode.Equals(HttpStatusCode.OK))
{
    ... return
}

next line

if (ReferenceEquals(response.ContentType, null) || !IsValidContentType(response.ContentType))
{
    ... return
}

and so on...

You can also reduce using nesting by putting them one below the other:

using (var stream = response.GetResponseStream())
using (var ms = new MemoryStream())//store stream to variable
{
    ... do stuff
}

here you can even remove one using as the MemoryStream can take the response stream as a parameter in its constructor.

I don't understand why you are copying this stream twice - for caching and for validating. You can do both, just rewind it after you've validated it with MemoryStream.Seek.