Skip to content
This repository was archived by the owner on Nov 20, 2018. It is now read-only.

Set default path=/ when removing cookie #476

Merged
merged 1 commit into from
Nov 16, 2015
Merged

Set default path=/ when removing cookie #476

merged 1 commit into from
Nov 16, 2015

Conversation

pakrym
Copy link
Contributor

@pakrym pakrym commented Nov 13, 2015

@dnfclas
Copy link

dnfclas commented Nov 13, 2015

Hi @pakrym, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

@halter73
Copy link
Member

:shipit:

@davidfowl
Copy link
Member

Weird we had that much logic that could be readily deleted 😄 /cc @Tratcher

@pakrym pakrym force-pushed the pakrym/remove-cookie branch from f0108cd to fa5d9fa Compare November 16, 2015 19:02
@Tratcher
Copy link
Member

@davidfowl It has been like that sense Katana, @lodejard did it as a perf optimization because the logic was much simpler for that scenario.

string cookieHeaderValue = headers[HeaderNames.SetCookie];
Assert.StartsWith(testcookie, cookieHeaderValue);
Assert.Contains("path=/", cookieHeaderValue);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also add a test where Append adds the cookie and then Delete removes it.

@pakrym
Copy link
Contributor Author

pakrym commented Nov 16, 2015

🆙 📅

@JunTaoLuo
Copy link
Contributor

:shipit:

1 similar comment
@davidfowl
Copy link
Member

:shipit:

@pakrym pakrym force-pushed the pakrym/remove-cookie branch from e840e31 to 690e5a6 Compare November 16, 2015 23:59
@pakrym pakrym merged commit 690e5a6 into dev Nov 16, 2015
@JunTaoLuo JunTaoLuo deleted the pakrym/remove-cookie branch January 6, 2017 22:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants