Skip to content
This repository was archived by the owner on Jul 9, 2023. It is now read-only.

Conversation

lmarszal
Copy link

Doneness:

  • Build is okay - I made sure that this change is building successfully.
  • No Bugs - I made sure that this change is working properly as expected. It doesn't have any bugs that you are aware of.
  • Branching - If this is not a hotfix, I am making this request against develop branch

Tried to run Titanium-Web-Proxy in docker. Run into two problems:

  1. Setting FriendlyName on X509Certificate2 is not supported on Linux - and there was a bug here that it was being set always for .net standard (see pull request)
  2. SNI is not supported for SslStream in .NET Core 2.0 on Linux. Will be added in .NET Core 2.1, as per this pull request: Add server-side SNI support dotnet/corefx#28278 - I'm willing to update wiki to let others know about this issue.

I've also decided to remove this doNotSetFriendlyName variable, as we know that FriendlyName is just not suppored on Linux and should do it only for Windows.

@AppVeyorBot
Copy link

@honfika
Copy link
Collaborator

honfika commented May 31, 2018

not suppored on Linux and should do it only for Windows
What about for Mac and Mono in Windows/Linux? .NET core on Windows?
Are you sure that it is correct everywhere?
I prefer to keep the current code, because it will work automatically when they implement this property.

I've removed the wrong property set line.

@lmarszal
Copy link
Author

I've removed the wrong property set line.

Thanks.

not suppored on Linux and should do it only for Windows
What about for Mac and Mono in Windows/Linux? .NET core on Windows?
Are you sure that it is correct everywhere?
I prefer to keep the current code, because it will work automatically when they implement this property.

Didn't test it on Mac or Mono. On .NET core windows works fine. And yes - current code is OK, I just don't like exception-driven logic. But it's only my preferences.
We can leave code as is (with your change mentioned above) and close this PR.

@lmarszal lmarszal closed this Jun 18, 2018
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