Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating ASP.NET Core references #5333

Merged
merged 1 commit into from
Dec 12, 2019
Merged

Updating ASP.NET Core references #5333

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/WebJobs.Script.WebHost/WebJobs.Script.WebHost.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@

<ItemGroup>
<PackageReference Include="DotNetTI.BreakingChangeAnalysis" Version="1.0.5-preview" />
<PackageReference Include="Microsoft.AspNetCore.App" Version="2.2.7" />
<PackageReference Include="Microsoft.AspNetCore.App" Version="2.2.8" />
<PackageReference Include="Microsoft.Azure.AppService.Proxy.Client" Version="2.0.8100001-0126c21e" />
<PackageReference Include="Microsoft.Azure.Services.AppAuthentication" Version="1.0.3" />
<PackageReference Include="Microsoft.Azure.WebJobs" Version="3.0.15" />
Expand Down
1 change: 0 additions & 1 deletion src/WebJobs.Script/Binding/Http/RawScriptResult.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ public async Task ExecuteResultAsync(ActionContext context)
{
foreach (var cookie in Cookies)
{
// Item3 (CookieOptions) should not be null, but this will behave correctly if it is
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: i think this comment still applies

Copy link
Member Author

Choose a reason for hiding this comment

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

Noticed this comment after merging. I removed as it is a handled scenario and I din't feel it was providing much value to others working on the codebase. Is there something you feel others may be tempted to do if this comment is not present?

Side note, it's unfortunate, but there's a slight difference in behavior between the two Append overloads.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's a good point :) I mainly added it just to say this is not an expected code path in case someone is trying to reason through what the inputs look like, but of course that could change. So I think it's fair to remove it!

if (cookie.Item3 != null)
{
response.Cookies.Append(cookie.Item1, cookie.Item2, cookie.Item3);
Expand Down
4 changes: 2 additions & 2 deletions src/WebJobs.Script/Workers/Rpc/MessageExtensions/RpcMessageExtensionUtilities.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,9 @@ private static SameSiteMode RpcSameSiteEnumConverter(RpcHttpCookie.Types.SameSit
case RpcHttpCookie.Types.SameSite.Lax:
return SameSiteMode.Lax;
case RpcHttpCookie.Types.SameSite.None:
return SameSiteMode.None;
return (SameSiteMode)(-1);
default:
return SameSiteMode.None;
return (SameSiteMode)(-1);
}
}
}
Expand Down
9 changes: 5 additions & 4 deletions test/WebJobs.Script.Tests/Binding/ActionResults/RawScriptResultTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,14 @@ public async Task AddsHttpCookies()
{
new Tuple<string, string, CookieOptions>("firstCookie", "cookieValue", new CookieOptions()
{
SameSite = SameSiteMode.None
fabiocav marked this conversation as resolved.
Show resolved Hide resolved
SameSite = SameSiteMode.Lax
}),
new Tuple<string, string, CookieOptions>("secondCookie", "cookieValue2", new CookieOptions()
{
Path = "/",
HttpOnly = true,
MaxAge = TimeSpan.FromSeconds(20)
MaxAge = TimeSpan.FromSeconds(20),
SameSite = (SameSiteMode)(-1)
})
}
};
Expand All @@ -79,8 +80,8 @@ public async Task AddsHttpCookies()
context.HttpContext.Response.Headers.TryGetValue("Set-Cookie", out StringValues cookies);

Assert.Equal(2, cookies.Count);
Assert.Equal("firstCookie=cookieValue; path=/", cookies[0]);
Assert.Equal("secondCookie=cookieValue2; max-age=20; path=/; samesite=lax; httponly", cookies[1]);
Assert.Equal("firstCookie=cookieValue; path=/; samesite=lax", cookies[0]);
Assert.Equal("secondCookie=cookieValue2; max-age=20; path=/; httponly", cookies[1]);
}

[Fact]
Expand Down