[ https://issues.apache.org/jira/browse/YARN-8198?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16456334#comment-16456334 ]
Szilard Nemeth commented on YARN-8198: -------------------------------------- - HttpServer2 (minor thing): As you only use {{getDefaultHeaders}} from {{setHeaders}}, I would put the {{getDefaultHeaders}} method under the {{setHeaders}} method. - HttpServer2: introduced constant Strings could be package private. - *HttpServer2 in general about regex usage: You use {{HTTP_HEADER_REGEX}} in 3 places: * - addHeaders - setHeaders (2 occurences) In {{addHeaders()}}, you could simply use {{String.startsWith()}} because AFAIK you don't validate with the regex, you just check whether the String starts with a prefix. In {{setHeaders()}}, you call {{conf.getValByRegex()}}, I think if you just leverage the startsWith capability of your regex, you could just call {{conf.getPropsWithPrefix()}} because it's more lightweight than a regex matching. If you still decide to keep regex-matching instead of simple String operations, I think you should fix these: - {{setHeaders()}}: you call {{conf.getValByRegex()}}, this method returns every config matching for a regex. Then you forEach on the {{headerConfigMap}} and do the same regex-matching that the {{getValByRegex}} did, which is not needed. You could just simply call {{xFrameParams.putAll(headerConfigMap);}} as the last step of this method. - {{setHeaders()}}: if you want to ensure that no config is present with name "hadoop.http.header.X", where X does not match for your regex ([a-zA-Z\-_]+), I think you should indicate the unmatched values with a log message (warning?). - You should call {{Pattern.compile(HTTP_HEADER_REGEX)}} only once and save it to a static final field of {{HttpServer2}}. You have 2 occurences of this pattern call, both in a loop, which is costly and unnecessarry to perform every time. - In your {{HTTP_HEADER_REGEX}}, I think the first regex group is not relevant as you are only interested in what follows "hadoop.http.header." I guess. - {{HttpServer2.addHeaders()}}: You have: {{String value = config.getInitParameter(key);]] You don't use the value local variable, I guess you wanted to use it on the next line. - {{HttpServer2.getDefaultHeaders()}}: You used split twice on both headers, you could store the split results in arrays and access the elements from 0th and 1st index. > Add Security-Related HTTP Response Header in Yarn WEBUIs. > --------------------------------------------------------- > > Key: YARN-8198 > URL: https://issues.apache.org/jira/browse/YARN-8198 > Project: Hadoop YARN > Issue Type: Improvement > Components: yarn > Reporter: Kanwaljeet Sachdev > Assignee: Kanwaljeet Sachdev > Priority: Major > Labels: security > Attachments: YARN-8198.001.patch, YARN-8198.002.patch, > YARN-8198.003.patch > > > As of today, YARN web-ui lacks certain security related http response > headers. We are planning to add few default ones and also add support for > headers to be able to get added via xml config. Planning to make the below > two as default. > * X-XSS-Protection: 1; mode=block > * X-Content-Type-Options: nosniff > > Support for headers via config properties in core-site.xml will be along the > below lines > {code:java} > <property> > <name>hadoop.http.header.Strict_Transport_Security</name> > <value>valHSTSFromXML</value> > </property>{code} > > A regex matcher will lift these properties and add into the response header > when Jetty prepares the response. -- This message was sent by Atlassian JIRA (v7.6.3#76005) --------------------------------------------------------------------- To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org