C# CookieParser for WebRequests

[C#] CookieParser for WebRequests - Printable Version

+- Hack Forums (https://hackforums.net)
+-- Forum: Tech (https://hackforums.net/forumdisplay.php?fid=444)
+--- Forum: Coding (https://hackforums.net/forumdisplay.php?fid=151)
+---- Forum: Software Development (https://hackforums.net/forumdisplay.php?fid=118)
+---- Thread: [C#] CookieParser for WebRequests (/showthread.php?tid=5771904)

Pages: Pages 1 2

CookieParser for WebRequests - Niǌa - Dec 20, 2017

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.

RE: CookieParser for WebRequests - BahNahNah - Dec 20, 2017

Why recreate the DateTime class?
[Image: AJn0RFQ.png]

RE: CookieParser for WebRequests - Niǌa - Dec 21, 2017

(Dec 20, 2017 11:14 PM)BahNahNah Wrote: 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.

RE: CookieParser for WebRequests - HFDone - Dec 21, 2017

(Dec 21, 2017 08:25 AM)Niǌa Wrote: 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 😖

RE: CookieParser for WebRequests - Niǌa - Dec 21, 2017

(Dec 21, 2017 11:44 AM)HFDone Wrote: [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.

RE: CookieParser for WebRequests - itcorp - Dec 21, 2017

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

RE: CookieParser for WebRequests - 0xDEAD10CC - Dec 22, 2017

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?

RE: CookieParser for WebRequests - Niǌa - Dec 22, 2017

(Dec 22, 2017 12:54 PM)0xDEAD10CC Wrote:
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.

RE: CookieParser for WebRequests - BahNahNah - Dec 22, 2017

(Dec 22, 2017 05:25 PM)Niǌa Wrote: 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?

RE: CookieParser for WebRequests - Niǌa - Dec 22, 2017

(Dec 22, 2017 05:48 PM)BahNahNah Wrote: 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.

RE: CookieParser for WebRequests - newmecha - Dec 22, 2017

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.

RE: CookieParser for WebRequests - BahNahNah - Dec 22, 2017

(Dec 22, 2017 08:04 PM)newmecha Wrote: 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.

RE: CookieParser for WebRequests - Niǌa - Dec 22, 2017

(Dec 22, 2017 08:04 PM)newmecha Wrote: - 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.

RE: CookieParser for WebRequests - newmecha - Dec 22, 2017

(Dec 22, 2017 08:08 PM)BahNahNah Wrote: 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.

RE: CookieParser for WebRequests - 0xDEAD10CC - Dec 24, 2017

(Dec 22, 2017 05:25 PM)Niǌa Wrote: 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/4a581147d9a248086250977a48b75441#file-cookieparser-cs-L80

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