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

Preserve secure and httponly headers in ResponseCookies #905

Merged
merged 1 commit into from
Aug 11, 2017

Conversation

jkotalik
Copy link
Contributor

#541 @Tratcher
I would like to manually verify the before and after of this change in the response cookie, @Tratcher can you help me set that up?

Copy link
Member

@Tratcher Tratcher left a comment

Choose a reason for hiding this comment

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

Let's try this in a real app. I'll show you how to do a cross repo build with your changes.

@@ -6,6 +6,7 @@
using Microsoft.Extensions.ObjectPool;
using Microsoft.Net.Http.Headers;
using Xunit;
using System;
Copy link
Member

Choose a reason for hiding this comment

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

sort System goes first. There's a VS setting for this.

@jkotalik
Copy link
Contributor Author

jkotalik commented Aug 9, 2017

@Tratcher Tested on a real application and the secure and httponly headers are preserved.

@@ -129,6 +129,8 @@ public void Delete(string key, CookieOptions options)
Path = options.Path,
Domain = options.Domain,
Expires = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc),
Secure = options.Secure,
HttpOnly = options.HttpOnly
Copy link
Member

Choose a reason for hiding this comment

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

Add the new SameSite option as well.

@jkotalik jkotalik force-pushed the jkotalik/ResponseCookies branch from 88b47e1 to 6dd8424 Compare August 10, 2017 23:46
@jkotalik jkotalik merged commit 594f559 into dev Aug 11, 2017
@jkotalik jkotalik deleted the jkotalik/ResponseCookies branch August 11, 2017 01:01
jkotalik added a commit that referenced this pull request Aug 25, 2017
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.

3 participants