Vojtech Szocs has posted comments on this change. Change subject: userportal,webadmin: token generation fix ......................................................................
Patch Set 1: (3 comments) http://gerrit.ovirt.org/#/c/30849/1//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2014-07-23 15:30:24 -0400 Line 4: Commit: Alexander Wels <aw...@redhat.com> Line 5: CommitDate: 2014-07-30 10:29:37 -0400 Line 6: Line 7: userportal,webadmin: token generation fix To make it more clear, please indicate that the token in question is XSRF token for GWT RPC calls. Line 8: Line 9: - Fix token generation to use session id instead of passed Line 10: in jsessionid cookie, as that value might be stale. This Line 11: prevents a lot of 500 errors in the log due to automatic http://gerrit.ovirt.org/#/c/30849/1/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/OvirtXsrfTokenServiceServlet.java File frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/OvirtXsrfTokenServiceServlet.java: Line 13: private static final long serialVersionUID = 1854606938563216502L; Line 14: Line 15: /** Line 16: * Generates and returns new XSRF token. Line 17: */ Shouldn't @Override be placed here? Also, no need to re-type the Javadoc; if it's missing, Javadoc will be taken from supertype method declaration. To extend supertype Javadoc, you can use: /** * extra stuff {@inheritDoc} extra stuff */ Line 18: public XsrfToken getNewXsrfToken() { Line 19: return new XsrfToken(generateTokenValueResponse()); Line 20: } Line 21: Line 19: return new XsrfToken(generateTokenValueResponse()); Line 20: } Line 21: Line 22: private String generateTokenValueResponse() { Line 23: byte[] cookieBytes = getThreadLocalRequest().getSession().getId().getBytes(); > Charset.forName("UTF-8")? Similar code exists in XsrfTokenServiceServlet#generateTokenValue Java runtime typically uses UTF-8 as fallback for default charset, see java.nio.charset.Charset#defaultCharset for details. Unless there is "file.encoding" Java system property defined when starting Java runtime, UTF-8 is (typically) the default charset. Since XsrfTokenServiceServlet passes the token to client via GWT RPC, and since GWT RPC protocol implies the use of UTF-8 encoding, the code above should work. As Alon mentioned, we could also mention UTF-8 charset explicitly, I am not against it. Alex, why is the variable called "cookieBytes"? (seems more like "sessionIdBytes" or similar) Also, why not name the method generateTokenValue() like the private one in superclass? Line 24: return StringUtils.toHexString(Md5Utils.getMd5Digest(cookieBytes)); Line 25: } -- To view, visit http://gerrit.ovirt.org/30849 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3e9a234bada73873f398d4220808f573810440dc Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Einav Cohen <eco...@redhat.com> Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
