[ https://issues.apache.org/jira/browse/HDFS-9711?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15144011#comment-15144011 ]
Larry McCay commented on HDFS-9711: ----------------------------------- Hey [~cnauroth] - I love the objectives of this and can see its elegance. I do also agree that it makes it hard to read. There is something especially odd about the indirection being completely built into the CSRF filter itself as well as the handler of the httpInteraction that makes you really wonder why is it done this way. I would find myself wanting to simplify it and not realize that some other component was using the handler method with another implementation of httpInteraction. You do however document the HttpInteraction pretty well in the same file. So, maybe it is fine. I am curious about the following line in the javadoc for the interface though: {quote} + * Typical usage of the filter inside a servlet container will not need to use + * this interface. {quote} The HttpInteraction is certainly being use from doFilter. Are you saying the there is no code other than the doFilter implementation that will need to use the HttpInteraction instance directly? That seems to make sense. Removing the anonymous extension may help make it more readable. Instead of: {quote} + handleHttpInteraction(new HttpInteraction() { + @Override + public String getHeader(String header) { + return httpRequest.getHeader(header); + } + + @Override + public String getMethod() { + return httpRequest.getMethod(); + } + + @Override + public void proceed() throws IOException, ServletException { + chain.doFilter(httpRequest, httpResponse); + } + + @Override + public void sendError(int code, String message) throws IOException { + httpResponse.sendError(code, message); + } + }); {quote} We created a concrete instance of the ServletFilterHttpInteraction like: {quote} handleHttpInteraction(new ServletFilterHttpInteraction(request, response, chain)); {quote} and: {quote} handleHttpInteraction(new NettyHttpInteraction(final ChannelHandlerContext ctx, final HttpRequest req)); {quote} Do you think it would help? > Integrate CSRF prevention filter in WebHDFS. > -------------------------------------------- > > Key: HDFS-9711 > URL: https://issues.apache.org/jira/browse/HDFS-9711 > Project: Hadoop HDFS > Issue Type: New Feature > Components: datanode, namenode, webhdfs > Reporter: Chris Nauroth > Assignee: Chris Nauroth > Attachments: HDFS-9711.001.patch, HDFS-9711.002.patch, > HDFS-9711.003.patch, HDFS-9711.004.patch > > > HADOOP-12691 introduced a filter in Hadoop Common to help REST APIs guard > against cross-site request forgery attacks. This issue tracks integration of > that filter in WebHDFS. -- This message was sent by Atlassian JIRA (v6.3.4#6332)