Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

[Fixes #4112 #4093] Adding support for custom SSL port #4113

Merged
merged 1 commit into from
Feb 23, 2016

Conversation

sebastienros
Copy link
Member

New optional MvcOptions.SslPort. If not defined the redirection uses an empty port (default),
otherwise the custom port is used.

@@ -6,6 +6,9 @@
using Microsoft.AspNetCore.Mvc.Abstractions;
using Microsoft.AspNetCore.Mvc.Filters;
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Options;
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.

Nit: sort using a properly

@sebastienros sebastienros force-pushed the sebros/requiressl branch 2 times, most recently from c943458 to 49112c0 Compare February 19, 2016 00:39
@sebastienros sebastienros changed the title [Fixes #4112 #911] Adding support for custom SSL port [Fixes #4112 #4093] Adding support for custom SSL port Feb 19, 2016
[InlineData("http://localhost:5000", null, "https://localhost/")]
[InlineData("http://[2001:db8:a0b:12f0::1]", null, "https://[2001:db8:a0b:12f0::1]/")]
[InlineData("http://[2001:db8:a0b:12f0::1]:5000", null, "https://[2001:db8:a0b:12f0::1]/")]
[InlineData("http://localhost:5000/path", null, "https://localhost/path")]
Copy link
Member

Choose a reason for hiding this comment

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

Can you add variation(s) having query string too?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @sebastienros Just FYI...you seem to have squashed your commits. This makes it difficult to find the differences(to see how the feedback was addressed) from earlier commit. You could squash the commits once you get a sign off.

@sebastienros sebastienros force-pushed the sebros/requiressl branch 2 times, most recently from 71893dd to 4440358 Compare February 22, 2016 19:21
@kichalla
Copy link
Member

:shipit:

@javiercn
Copy link
Member

Looks good to me, :shipit:

@@ -16,6 +19,8 @@ public class RequireHttpsAttributeTests
public void OnAuthorization_AllowsTheRequestIfItIsHttps()
{
// Arrange
var options = new TestOptionsManager<MvcOptions>();
Copy link
Member

Choose a reason for hiding this comment

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

Remove. This isn't used.

@dougbu
Copy link
Member

dougbu commented Feb 23, 2016

:shipit:

New optional MvcOptions.SslPort. If not defined the redirection uses an empty port (default),
otherwise the custom port is used.
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.

7 participants