----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69916/#review212626 -----------------------------------------------------------
core/src/main/java/org/apache/oozie/servlet/HTTPResponseFilter.java Lines 36 (patched) <https://reviews.apache.org/r/69916/#comment298447> Please give this a more descriptive and specific name like `HttpResponseHeadersFilter`. core/src/main/java/org/apache/oozie/servlet/HTTPResponseFilter.java Lines 69-70 (patched) <https://reviews.apache.org/r/69916/#comment298448> Shouldn't we first call `chain.doFilter()`, and then add the response header(s) to possibly avoid setting them to `null` in a later call down the filter chain? core/src/test/java/org/apache/oozie/servlet/TestHTTPResponseFilter.java Lines 36 (patched) <https://reviews.apache.org/r/69916/#comment298450> While this only test case is OK with me, can you please add more to this: * clickjacking attempt should result unsuccessful * normal HTTP servlet, e.g. `VersionServlet`, gives also this HTTP response header core/src/test/java/org/apache/oozie/servlet/TestHTTPResponseFilter.java Lines 39 (patched) <https://reviews.apache.org/r/69916/#comment298449> Please rename to `testXFrameOptionAgainstClickjackingAdded()`. core/src/test/java/org/apache/oozie/servlet/TestHTTPResponseFilter.java Lines 44 (patched) <https://reviews.apache.org/r/69916/#comment298446> Why do we need `AtomicBoolean` here? - András Piros On Feb. 7, 2019, 12:40 p.m., Andras Salamon wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69916/ > ----------------------------------------------------------- > > (Updated Feb. 7, 2019, 12:40 p.m.) > > > Review request for oozie, Denes Bodo, Kinga Marton, and Mate Juhasz. > > > Repository: oozie-git > > > Description > ------- > > OOZIE-3427 - Use best practices in HTTP response headers > > > Diffs > ----- > > core/src/main/java/org/apache/oozie/servlet/HTTPResponseFilter.java > PRE-CREATION > core/src/test/java/org/apache/oozie/servlet/TestHTTPResponseFilter.java > PRE-CREATION > server/src/main/java/org/apache/oozie/server/FilterMapper.java 3dc9be815 > webapp/src/main/webapp/WEB-INF/web.xml 2edbdf153 > > > Diff: https://reviews.apache.org/r/69916/diff/1/ > > > Testing > ------- > > Tested embedded jetty and war installed to a local tomcat. Local tomcat was > hardly working, but at least I was able to test /versions. > > $ wget -qS http://localhost:11000/oozie/index.jsp > HTTP/1.1 200 OK > Date: Thu, 07 Feb 2019 09:44:32 GMT > X-Frame-Options: DENY > Content-Type: text/html;charset=utf-8 > Set-Cookie: JSESSIONID=1lx0y9fy2pd6n1rh911vc2l1sd;Path=/oozie > Expires: Thu, 01 Jan 1970 00:00:00 GMT > Content-Length: 3739 > > > Thanks, > > Andras Salamon > >
