Handle parsing port in host when reading V2#289
Conversation
| host = "//" + host; // The double slash prefix creates a relative url where the scheme is defined by the BaseUrl | ||
| } | ||
|
|
||
| int? port = null; |
There was a problem hiding this comment.
int? [](start = 20, length = 4)
nullable not needed -- int.Parse(pieces.Last()) throws an error if pieces.Last() is not parsable.
There was a problem hiding this comment.
@PerthCharern I think my logic is correct here. If the host does not contain ":" then we won't try to call int.Parse(pieces.Last()) and therefore port will be null. I don't want to update uriBuilder.Port if port isn't specified because the URL should omit port.
If the host does contain ":" and the part after the colon is not a number, I'm ok with it throwing a parsing exception.
| @@ -149,39 +149,19 @@ private static void MakeServers(IList<OpenApiServer> servers, ParsingContext con | |||
| { | |||
There was a problem hiding this comment.
If schemes is set to an empty list, I think we should treat that in the same way as schemes being null, i.e. line 148 should be if (schemes != null && schemes.Count != 0)
Spec says:
If the schemes is not included, the default scheme to be used is the one used to access the Swagger definition itself.
The term "not included" is not really precisely defined, but I think it should also apply to an empty schemes list.
If we don't modify the if clause as above, we won't have any server when schemes is an empty list. When servers is an empty list, the V3 spec interprets it as a default url of /, which is something totally different.
|
I approved, but please see the above comment before checking in. |
2 participants
No description provided.