Automatically decode cookie values in Cookies dictionary #1390

richardprice · 2014-01-03T14:48:35Z

A new test to check if the cookie value is correctly decoded on reading
from Request.Cookies and a fix for the existing issue the test
highlights.

A new test to check if the cookie value is correctly decoded on reading from Request.Cookies and a fix for the existing issue the test highlights.

jchannon · 2014-01-03T14:53:32Z

src/Nancy.Tests.Functional/Tests/CookieTests.cs

+        [Fact]
+        public void Cookie_should_decode_value_correctly()
+        {
+            // When


Can you make it Given & When

Broke out cookie test module into its own file and rewrote test into Given When format.

thecodejunkie · 2014-01-21T07:42:07Z

@richardprice Travis does not like this one. Tell me something's gone boo-boo

Nancy.Testing.Tests.BrowserFixture.Should_maintain_cookies_even_if_not_set_on_directly_preceding_request [FAIL]
   Assert.Equal() Failure
   Position: First difference is at position 26
   Expected: Current session value is: I've created a session!
   Actual:   Current session value is:

thecodejunkie · 2014-01-27T08:00:42Z

Still had failing tests when I ran it locally. Moved this to 0.23 because maybe we need to remake so that the cookie behavior is consistent between Request (IDictionary<string, string>) and Response (IList<INancyCookie>)

Thanks VS2012 for hiding these on Friday, for whatever reason you chose to.

richardprice · 2014-01-30T16:22:12Z

Do you want me to go ahead and start on the normalisation between Request and Response cookie handling?

thecodejunkie · 2014-02-05T19:34:51Z

@richardprice I think we need to discuss this first. Will talk to @grumpydev when the dust of the 0.22 release has settled

grumpydev · 2014-04-08T12:38:09Z

src/Nancy/Request.cs

@@ -177,7 +177,8 @@ public string Path
                    }
                }

-                cookieDictionary[cookieName] = parts[1];
+                cookieDictionary[cookieName] = Helpers.HttpUtility.UrlDecode(parts[1]);
+                //cookieDictionary[cookieName] = parts[1];


Commented out code? :)

thecodejunkie · 2015-08-22T10:32:46Z

Moving this to 1.4 milestone. @richardprice could you rebase so we have a running version to discuss on? Thanks ❤️

jchannon · 2015-09-24T13:46:19Z

ping @richardprice

richardprice · 2015-09-24T13:47:18Z

Sorry, missed this first time round, will sort soon.

On 24 September 2015 at 14:46, Jonathan Channon [email protected]
wrote:

ping @richardprice https://github.com/richardprice

—
Reply to this email directly or view it on GitHub
#1390 (comment).

The most exciting phrase to hear in science, the one that heralds new
discoveries, is not 'Eureka!' but 'That's funny...' - Isaac Asimov

A new test to check if the cookie value is correctly decoded on reading from Request.Cookies and a fix for the existing issue the test highlights.

Broke out cookie test module into its own file and rewrote test into Given When format.

Thanks VS2012 for hiding these on Friday, for whatever reason you chose to.

…Spike Conflicts: src/Nancy.Hosting.Wcf.Tests/NancyWcfGenericServiceFixture.cs src/Nancy.sln.DotSettings src/Nancy/Request.cs src/Nancy/Session/CookieBasedSessions.cs

khellang · 2015-10-28T19:50:19Z

This is good to go now?

richardprice · 2015-10-28T20:26:43Z

Yup.

On 28 October 2015 at 19:50, Kristian Hellang [email protected]
wrote:

This is good to go now?

—
Reply to this email directly or view it on GitHub
#1390 (comment).

The most exciting phrase to hear in science, the one that heralds new
discoveries, is not 'Eureka!' but 'That's funny...' - Isaac Asimov

khellang · 2015-10-28T21:08:02Z

src/Nancy.Tests.Functional/Tests/CookieTests.cs

+    using Testing;
+    using Xunit;
+
+    public class CookieTestsFixture


@richardprice Can you either 1) rename the class, or 2) rename the file? 😄 And maybe not include both "tests" and "fixture"?

Added test and fix for cookie decoding issue

khellang · 2015-10-28T22:14:21Z

❤️ Sorry it took almost 2 years 😝

khellang · 2015-10-30T14:09:48Z

This is a breaking change.

If you manually decode cookies from the Request.Cookies dictionary, you should remove the decoding:

- var cookie = HttpUtility.UrlDecode(request.Cookies[cookieName]);
+ var cookie = request.Cookies[cookieName];

Added test and fix for cookie decoding issue

Added test and fix for cookie decoding issue

776e756

A new test to check if the cookie value is correctly decoded on reading from Request.Cookies and a fix for the existing issue the test highlights.

jchannon reviewed Jan 3, 2014
View reviewed changes

Code tidy as per feedback from PR

89b4cfa

Broke out cookie test module into its own file and rewrote test into Given When format.

Ensured all tests pass after cookie decoding fix

5aa83d8

Found additional tests and ensured they pass

9c7512c

Thanks VS2012 for hiding these on Friday, for whatever reason you chose to.

thecodejunkie modified the milestones: 0.24, 0.23 Mar 16, 2014

grumpydev reviewed Apr 8, 2014
View reviewed changes

grumpydev modified the milestones: Future, 0.24 Jun 10, 2014

khellang removed this from the Future milestone Aug 19, 2015

thecodejunkie added this to the 1.4 milestone Aug 22, 2015

thecodejunkie modified the milestone: 1.4 Oct 23, 2015

richardprice added 7 commits October 28, 2015 18:11

Merge remote-tracking branch 'NancyFx/master'

9d53bea

Added test and fix for cookie decoding issue

76ac33f

A new test to check if the cookie value is correctly decoded on reading from Request.Cookies and a fix for the existing issue the test highlights.

Code tidy as per feedback from PR

5a937a6

Broke out cookie test module into its own file and rewrote test into Given When format.

Ensured all tests pass after cookie decoding fix

2025d14

Found additional tests and ensured they pass

7826278

Thanks VS2012 for hiding these on Friday, for whatever reason you chose to.

Fixed failing tests

e6284b3

Merge remote-tracking branch 'origin/CookieTestSpike' into CookieTest…

76cb199

…Spike Conflicts: src/Nancy.Hosting.Wcf.Tests/NancyWcfGenericServiceFixture.cs src/Nancy.sln.DotSettings src/Nancy/Request.cs src/Nancy/Session/CookieBasedSessions.cs

khellang reviewed Oct 28, 2015
View reviewed changes

richardprice added 2 commits October 28, 2015 21:34

Cleaned up PR

90d837a

More clean up as suggested by @khellang

b3283eb

khellang added a commit that referenced this pull request Oct 28, 2015

Merge pull request #1390 from richardprice/CookieTestSpike

c101d7b

Added test and fix for cookie decoding issue

khellang merged commit c101d7b into NancyFx:master Oct 28, 2015

khellang added Bug Breaking Change labels Oct 29, 2015

khellang changed the title ~~Added test and fix for cookie decoding issue~~ Automatically decode cookie values in Cookies dictionary Oct 30, 2015

fxbit pushed a commit to yodiwo/Nancy that referenced this pull request Aug 19, 2016

Merge pull request NancyFx#1390 from richardprice/CookieTestSpike

00f1653

Added test and fix for cookie decoding issue

khellang mentioned this pull request Aug 31, 2017

Upgrade to 1.4 causes fault on deployment #2093

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Automatically decode cookie values in Cookies dictionary #1390

Automatically decode cookie values in Cookies dictionary #1390

richardprice commented Jan 3, 2014

jchannon Jan 3, 2014

thecodejunkie commented Jan 21, 2014

thecodejunkie commented Jan 27, 2014

richardprice commented Jan 30, 2014

thecodejunkie commented Feb 5, 2014

grumpydev Apr 8, 2014

thecodejunkie commented Aug 22, 2015

jchannon commented Sep 24, 2015

richardprice commented Sep 24, 2015

khellang commented Oct 28, 2015

richardprice commented Oct 28, 2015

khellang Oct 28, 2015

khellang commented Oct 28, 2015

khellang commented Oct 30, 2015

Automatically decode cookie values in Cookies dictionary #1390

Automatically decode cookie values in Cookies dictionary #1390

Conversation

richardprice commented Jan 3, 2014

jchannon Jan 3, 2014

Choose a reason for hiding this comment

thecodejunkie commented Jan 21, 2014

thecodejunkie commented Jan 27, 2014

richardprice commented Jan 30, 2014

thecodejunkie commented Feb 5, 2014

grumpydev Apr 8, 2014

Choose a reason for hiding this comment

thecodejunkie commented Aug 22, 2015

jchannon commented Sep 24, 2015

richardprice commented Sep 24, 2015

khellang commented Oct 28, 2015

richardprice commented Oct 28, 2015

khellang Oct 28, 2015

Choose a reason for hiding this comment

khellang commented Oct 28, 2015

khellang commented Oct 30, 2015