Improve exception when HTTP.sys can't register a prefix by 0xced · Pull Request #20718 · dotnet/aspnetcore

0xced · 2020-04-10T10:03:31Z

Propose an actionable solution and include a link to the documentation.

@guardrex Can you create an aka.ms link to https://docs.microsoft.com/aspnet/core/fundamentals/servers/httpsys#configure-windows-server that I could use in the exception message?

guardrex · 2020-04-10T11:19:54Z

I can't ... I don't have access ... but @Rick-Anderson might help or advise.

halter73 · 2020-04-15T20:29:13Z

Can we add a test for this?

Is Environment.UserName in the right format to insert directly into the command?

@halter73 I added a test in 52c0eff.
@Tratcher I included both the domain + user in DOMAIN\User format, as documented on Configuring namespace reservations, also in 52c0eff.

Propose an actionable solution and include a link to the documentation.

0xced · 2020-04-23T20:02:35Z

I addressed the concerns raised by @shirhatti, @halter73 and @Tratcher, rebased onto master and force-pushed 52c0eff.

Tratcher · 2020-04-23T23:28:37Z

The new test failed on helix.
https://dev.azure.com/dnceng/public/_build/results?buildId=614229&view=ms.vss-test-web.build-test-results-tab&runId=19274364&resultId=107864&paneView=debug

Assert.Throws() Failure

Expected: typeof(Microsoft.AspNetCore.Server.HttpSys.HttpSysException)

Actual: (No exception was thrown)

You wouldn't get AccessDenied if the test were run as admin, right? I wonder if Helix does that. @HaoK ?

shirhatti · 2020-04-24T00:08:49Z

You could change the hostname in the test to localhost though? You wouldn't need elevation for that

Tratcher · 2020-04-24T00:20:37Z

The point is that the test wants AccessDenied and isn't getting it.

HaoK · 2020-04-24T01:19:59Z

So this is failing because of a sudo issue?

HaoK · 2020-04-24T01:56:11Z

Yeah so our helix jobs do run as admin, so if the tests were expecting admin access failures, you might need to skip this test on helix

ghost · 2020-04-24T17:40:32Z

Hello @Tratcher!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

HaoK · 2020-04-24T18:30:51Z

        }

+        [ConditionalFact]
+        [SkipOnHelix("https://github.com/dotnet/aspnetcore/pull/20718#issuecomment-618758634")]


You can target specific queues, if this only is an issue on Windows queues, it might be best to use the OS skip on windows

All HttpSys tests only run on Windows.

Tratcher · 2020-04-24T23:01:07Z

Unrelated broken test: #21191

0xced requested review from Tratcher, analogrelay, halter73 and jkotalik as code owners April 10, 2020 10:03

ghost added the area-servers label Apr 10, 2020

0xced force-pushed the 0xced/httpsys-improved-exception branch from 824f8cf to 9f4b507 Compare April 12, 2020 23:16

halter73 reviewed Apr 15, 2020

View reviewed changes

Comment thread src/Servers/HttpSys/src/Resources.resx Outdated

shirhatti reviewed Apr 15, 2020

View reviewed changes

Comment thread src/Servers/HttpSys/src/Resources.resx Outdated

0xced force-pushed the 0xced/httpsys-improved-exception branch from 9f4b507 to 6e43a83 Compare April 16, 2020 06:13

analogrelay added this to the 5.0.0-preview5 milestone Apr 20, 2020

0xced force-pushed the 0xced/httpsys-improved-exception branch 2 times, most recently from 9a0943f to 8784857 Compare April 23, 2020 19:59

Improve exception when HTTP.sys can't register a prefix

52c0eff

Propose an actionable solution and include a link to the documentation.

0xced force-pushed the 0xced/httpsys-improved-exception branch from 8784857 to 52c0eff Compare April 23, 2020 20:02

Tratcher self-assigned this Apr 23, 2020

Add SkipOnHelix attribute

53b173b

Tratcher approved these changes Apr 24, 2020

View reviewed changes

Tratcher added the auto-merge label Apr 24, 2020

HaoK reviewed Apr 24, 2020

View reviewed changes

Tratcher closed this Apr 27, 2020

Tratcher reopened this Apr 27, 2020

ghost merged commit b73b202 into dotnet:master Apr 27, 2020

0xced deleted the 0xced/httpsys-improved-exception branch April 28, 2020 06:46

Tratcher mentioned this pull request Sep 21, 2020

Server_RegisterUnavailablePrefix_ThrowsActionableHttpSysException #26156

Closed

amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 6, 2023

This pull request was closed.

Conversation

0xced commented Apr 10, 2020

Uh oh!

guardrex commented Apr 10, 2020

Uh oh!

halter73 Apr 15, 2020

Choose a reason for hiding this comment

Uh oh!

Tratcher Apr 16, 2020

Choose a reason for hiding this comment

Uh oh!

0xced Apr 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

0xced commented Apr 23, 2020

Uh oh!

Tratcher commented Apr 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shirhatti commented Apr 24, 2020

Uh oh!

Tratcher commented Apr 24, 2020

Uh oh!

HaoK commented Apr 24, 2020

Uh oh!

HaoK commented Apr 24, 2020

Uh oh!

ghost commented Apr 24, 2020

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

Uh oh!

HaoK Apr 24, 2020

Choose a reason for hiding this comment

Uh oh!

Tratcher Apr 24, 2020

Choose a reason for hiding this comment

Uh oh!

Tratcher commented Apr 24, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

0xced Apr 23, 2020 •

edited

Loading

Tratcher commented Apr 23, 2020 •

edited

Loading

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.