[
https://issues.apache.org/jira/browse/HADOOP-16718?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16978836#comment-16978836
]
Siyao Meng commented on HADOOP-16718:
-------------------------------------
[~avijayan] Thanks for the patch.
1. Would you rename {{SNI_HOST_CHECK_ENABLED_KEY}} to
{{HTTP_SNI_HOST_CHECK_ENABLED_KEY}} ? Just to make it consistent with other
http keys like {{HTTP_SELECTOR_COUNT_KEY}}. (Unfortunately
{{FILTER_INITIALIZER_PROPERTY}} is not a good example.)
2. The new config property needs to be documented in {{core-default.xml}}
3. Would you define {{HTTP_SNI_HOST_CHECK_ENABLED_DEFAULT = false}} and use
that instead in the following snippet in your patch:
{code}
+ if (!sniHostCheckEnabled) {
+ sniHostCheckEnabled = conf.getBoolean(SNI_HOST_CHECK_ENABLED_KEY,
false);
+ }
{code}
could be:
{code}
+ if (!sniHostCheckEnabled) {
+ sniHostCheckEnabled = conf.getBoolean(HTTP_SNI_HOST_CHECK_ENABLED_KEY,
HTTP_SNI_HOST_CHECK_ENABLED_DEFAULT);
+ }
{code}
4. I notice you added {{enableSniHostCheck()}}, that's great. But would you
rename and change its definition a bit:
{code}
+ /**
+ * Enable sniHostCheck for Https.
+ * @return Builder.
+ */
+ public Builder enableSniHostCheck() {
+ this.sniHostCheckEnabled = true;
+ return this;
+ }
{code}
into something like
{code}
+ /**
+ * Enable or disable sniHostCheck.
+ * @param sniHostCheckEnabled Enable sniHostCheck if true, else disable it.
+ * @return Builder.
+ */
+ public Builder setSniHostCheckEnabled(boolean sniHostCheckEnabled) {
+ this.sniHostCheckEnabled = sniHostCheckEnabled;
+ return this;
+ }
{code}
5. I'm happy with all the existing hadoop-common tests passing. Don't think we
need another unit test for this. [~weichiu] Thoughts?
> Allow disabling Server Name Indication (SNI) for Jetty
> ------------------------------------------------------
>
> Key: HADOOP-16718
> URL: https://issues.apache.org/jira/browse/HADOOP-16718
> Project: Hadoop Common
> Issue Type: Improvement
> Affects Versions: 3.2.1
> Reporter: Siyao Meng
> Assignee: Aravindan Vijayan
> Priority: Major
> Fix For: 3.2.2
>
> Attachments: HADOOP-16718-branch-3.2-v000.patch
>
>
> As of now, {{createHttpsChannelConnector()}} enables SNI by default with
> Jetty:
> {code}
> private ServerConnector createHttpsChannelConnector(
> Server server, HttpConfiguration httpConfig) {
> httpConfig.setSecureScheme(HTTPS_SCHEME);
> httpConfig.addCustomizer(new SecureRequestCustomizer());
> ServerConnector conn = createHttpChannelConnector(server, httpConfig);
> {code}
> with the default constructor without any parameters automatically setting
> {{sniHostCheck}} to {{true}}:
> {code}
> public SecureRequestCustomizer()
> {
> this(true);
> }
> {code}
> Proposal: We should make this configurable and probably default this to false.
> Credit: Aravindan Vijayan
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
