Expose api/status with HTTP + fix functional tests #4338
Conversation
skofman1
requested review from
joelverhagen,
agr,
loic-sharma,
xavierdecoster,
ryuyu,
chenriksson,
cristinamanum and
scottbommarito
| @@ -106,7 +106,8 @@ public static void Configuration(IAppBuilder app) | |||
| } | |||
| else | |||
| { | |||
| app.UseForceSsl(config.Current.SSLPort, new[] { config.Current.ForceSslExclusion }); | |||
| var paths = config.Current.ForceSslExclusion.Split(';'); | |||
There was a problem hiding this comment.
Today, this code does not add an empty string or whitespace exclusion. This is now possible with a config value of ;;;; or something like that. We should filter out empty or whitespace before passing paths into UseForceSsl.
There was a problem hiding this comment.
This is a configuration property that should only be used by people that know what they are doing (or even better, should never be changed). Given this, I don't think we should do excessive correctness checks on the input. If we start with ;;; we can also check the path has a valid format, etc. All are unnecessary.
| /// A string containing a path exluded from | ||
| /// forcing the HTTP to HTTPS redirection. | ||
| /// A string containing a path exluded from forcing the HTTP to HTTPS redirection. | ||
| /// To provide multiple paths separate them with ; | ||
| /// </summary> | ||
| [DefaultValue("")] | ||
| public string ForceSslExclusion { get; set; } |
There was a problem hiding this comment.
Can this type be a IEnumerable<string>? It looks like we have some other complex types below like MailAddress enabled by a TypeConverter.
There was a problem hiding this comment.
I checked this option, and we currently don't support arrays as part of configuration, and using TypeConverter for this will introduce unnecessary complexity.
There was a problem hiding this comment.
Any consumer of IAppConfiguration therefore has to perform the semicolon splitting and filtering out of useless whitespaces. The semicolon thing is essentially a workaround due to the fact that we can't express enumeration in our current config format. The runtime interaction with the config values should be as simple as possible.
(take the example of MailAddress, which is not just a simple string)
Fixes: #4336
Noticed HttpToHttps functional tests don't run during deployment, so fixed it.