Improve missing/malformed file handling in user-jwts #42309
Conversation
ghost
added
the
area-commandlinetools
captainsafia
added
old-area-web-frameworks-do-not-use
| @@ -123,7 +123,7 @@ public static void Register(ProjectCommandLineApplication app) | |||
| var name = nameOption.HasValue() ? nameOption.Value() : Environment.UserName; | |||
| optionsString += $"{Resources.JwtPrint_Name}: {name}{Environment.NewLine}"; | |||
|
|
|||
| var audience = audienceOption.HasValue() ? audienceOption.Values : DevJwtCliHelpers.GetAudienceCandidatesFromLaunchSettings(project).ToList(); | |||
| var audience = audienceOption.HasValue() ? audienceOption.Values : DevJwtCliHelpers.GetAudienceCandidatesFromLaunchSettings(project)?.ToList(); | |||
| optionsString += audienceOption.HasValue() ? $"{Resources.JwtPrint_Audiences}: {audience}{Environment.NewLine}" : string.Empty; | |||
There was a problem hiding this comment.
| optionsString += audienceOption.HasValue() ? $"{Resources.JwtPrint_Audiences}: {audience}{Environment.NewLine}" : string.Empty; | |
| optionsString += audience is not null ? $"{Resources.JwtPrint_Audiences}: {audience}{Environment.NewLine}" : string.Empty; |
This would then print the URL gotten from launchSettings which we weren't including before (bug creep 😆)
There was a problem hiding this comment.
That was actually intentional. This was meant to only display options that were explicitly passed by the user, not ones we implicitly derived (e.g. username from Environment.UserName or urls from launchSettings).
|
So does the tool correctly discover all URLs from the launchSettings.json file now? The IIS Express URL? |
Yes, as of 9ff265e. |
| } | ||
| applicationUrls.AddRange(kestrelUrls); | ||
| } | ||
|
|
There was a problem hiding this comment.
I don't think that's the right call here. We want to populate audience from both IIS and Kestrel-based URLs here so we get a complete set of audiences by the end.
There was a problem hiding this comment.
The reason I think it should be else if is because if the Kestrel check of profile.Value.TryGetProperty("commandName", out var commandName) passes then the IIS check won't pass for this specific profile. Again, it's a nit, not code we need to worry about performance for.
| List<string> iisUrls = new(); | ||
| if (iisExpress.TryGetProperty("applicationUrl", out var iisUrl)) | ||
| { | ||
| iisUrls.Add(iisUrl.GetString()); |
There was a problem hiding this comment.
can these urls also be ; separated?
There was a problem hiding this comment.
I didn't think so, but we'd need to check with VS
There was a problem hiding this comment.
I tested it out. VS will throw if you have something like this:
"iisExpress": {
"applicationUrl": "http://localhost:22686;http://localhost:22687",
"sslPort": 44333
}
amcasey
added
the
area-commandlinetools
Closes #42306
Closes #42304