[Ninja]

So a user called VEN0M+ posted that he was having a problem with sessions on the Spotify login page. Turns out that the WebResponse does not parse the cookies correctly, as the Set-Cookie header did have the CSRF cookie in it, but the CookieContainer was empty no matter what we tried. Since I heard of this problem before, and since I experienced it, I decided to write a cookie parser myself.

You can find the source here.

The code follows section 5.2 of RFC 6265, and I tested it as much as I could with all cases documented in the RFC, but should you find a bug please let me know.

[BahNahNah]

Why recreate the DateTime class?

[Ninja]

Why recreate the DateTime class?

I didn't know it worked that way...
Man I feel stupid now. Let me edit OP and no one will ever see.

[HFDone]

I didn't know it worked that way...
Man I feel stupid now. Let me edit OP and no one will ever see.

[4+ years experience] C#/VB.NET Programming Service  😧 again  😖

[Ninja]

[4+ years experience] C#/VB.NET Programming Service  😧 again  😖

Next time when you forget your wallet at home remember you have 50 years of experience in life and that you are a waste of space and oxygen and should never have been born.

See, works the other way around.

[itcorp]

Nice sharing Ninja,as always..You definitely give good things...

[0xDEAD10CC]

Code

private void SetVersion(Cookie cookie, string value)
{
    if (!Char.IsNumber(value[0]))
        return;

    if (!int.TryParse(value, out int version))
        return;

    cookie.Version = version;
}

Why the preliminary check and no success indicator?

Code

private bool SetVersion(Cookie cookie, string value)
{
    if (int.TryParse(value, out int version))
    {
        cookie.Version = version;
        return true;
    }
    return false;
}

Only had a couple mins to skim through this, though if it only accepts integers why not take an integer rather than a string?

[Ninja]

Code

private void SetVersion(Cookie cookie, string value)
{
   if (!Char.IsNumber(value[0]))
       return;

   if (!int.TryParse(value, out int version))
       return;

   cookie.Version = version;
}

Why the preliminary check and no success indicator?

Code

private bool SetVersion(Cookie cookie, string value)
{
   if (int.TryParse(value, out int version))
   {
       cookie.Version = version;
       return true;
   }
   return false;
}

Only had a couple mins to skim through this, though if it only accepts integers why not take an integer rather than a string?

The RFC stated that should it not match it should be ignored, so that is what I did. I could have made it return a boolean, but it is not something the end user would or should get notified of, nor does it matter in the rest of the code whether or not it was successful, so I decided not to.

[BahNahNah]

The RFC stated that should it not match it should be ignored, so that is what I did. I could have made it return a boolean, but it is not something the end user would get notified, nor does it matter in the rest of the code whether or not it was successful, so I decided not to.

whats with the 

Code

if (!Char.IsNumber(value[0]))

           return;

meme?

If value is null/empty, its gonna through you a juicy error. Why not just TryParse to int?

[Ninja]

whats with the 

Code

if (!Char.IsNumber(value[0]))

           return;

meme?

If value is null/empty, its gonna through you a juicy error. Why not just TryParse to int?

You are right, I missed that. Let me fix it.

EDIT: Fixed.

[newmecha]

Code

private void SetVersion(Cookie cookie, string value)
   {
       if (string.IsNullOrEmpty(value) || !Char.IsNumber(value[0]))
           return;

       if (!int.TryParse(value, out int version))
           return;

       cookie.Version = version;
   }

The following check is redundant, that's the whole point of int.TryParse. If it can not be converted to an int return false, if null return false.

Code

if (string.IsNullOrEmpty(value) || !Char.IsNumber(value[0]))
               return;

I mean the assumption is that the first char in value is a number, what happens if 0T is passed ? it makes the check redundant sense int.TryParse would catch this anyway.

Can be simplified to just

Code

       private void SetVersion(Cookie cookie, string value)
       {
           if (!int.TryParse(value, out var version))
               return;
           cookie.Version = version;
       }

You should check if cookie header is null, let alone can be split.

Code

public Cookie ParseCookie(string cookieHeader, Uri responseUri)
   {
       string[] rawCookie = cookieHeader.Split(new char[] { ';' }, StringSplitOptions.RemoveEmptyEntries);
       string[] property = rawCookie[0].Split(new char[] { '=' }, StringSplitOptions.RemoveEmptyEntries);
.......

Can't split on null, and it's assuming it will have at lest 1 entry is there, so even if its not null but can't be split if it has no entries it will crash there also.

[BahNahNah]

Can't split on null, and it's assuming it will have at lest 1 entry is there, so even if its not null but can't be split if it has no entries it will crash there also.

You can split if the delimiter dosent exist. It returns an array with one first item as the whole string.

[Ninja]

- LONG QUOTE -

First of all, that is what the RFC documented. And the RFC defines the standard that most, if not all, browsers nowadays use so I followed it closely. But you are right, it is essentially the same, I'll fix it. And there really can't be a null in the cookie header unless the server fucks up majorly, but I guess I could add a check there. Thanks for the findings.

EDIT: Glanced over it again and fixed some more bugs I found.

[newmecha]

You can split if the delimiter dosent exist. It returns an array with one first item as the whole string.

Touche on that and that's correct it will return as the first entry.

[0xDEAD10CC]

The RFC stated that should it not match it should be ignored, so that is what I did. I could have made it return a boolean, but it is not something the end user would or should get notified of, nor does it matter in the rest of the code whether or not it was successful, so I decided not to.

Library developers commonly write and use diagnostic information into the code regardless on whether it'll be available to the end-user or not. As for the char isdigit thing, you could have avoided it and went with NumberStyles to properly use TryParse() if you want to avoid validating the value with leading whitespace.

int.TryParse() if you used it properly would handle a null value and leading whitespace for you... Making lines 82-87 redundant. https://gist.github.com/GekkeVent/4a5811...ser-cs-L80

int.TryParse(... NumberStyles.AllowLeadingSign ...)