Vojtech Szocs has posted comments on this change. Change subject: userportal,webadmin: xsrf token changes ......................................................................
Patch Set 1: (2 comments) http://gerrit.ovirt.org/#/c/31089/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 36: * Constructor that initializes the XSRF token generator. Line 37: * @throws NoSuchProviderException Line 38: * @throws NoSuchAlgorithmException Line 39: */ Line 40: OvirtXsrfTokenServiceServlet() throws NoSuchProviderException, NoSuchAlgorithmException { Hm, doesn't Java EE servlet spec require that each servlet has *public* no-arg constructor (either implicit if missing, or explicit) ? I suggest to move this kind of initialization into servlet's init() method which is called before an instance of a servlet is put to use. Line 41: super(); Line 42: random = SecureRandom.getInstance("SHA1PRNG", "SUN"); //$NON-NLS-1$ $NON-NLS-2$ Line 43: //Call nextBytes this once to initialize the random implementation. Line 44: //We ignore the result, we just called it to initialize the implementation. Line 37: * @throws NoSuchProviderException Line 38: * @throws NoSuchAlgorithmException Line 39: */ Line 40: OvirtXsrfTokenServiceServlet() throws NoSuchProviderException, NoSuchAlgorithmException { Line 41: super(); In Java language, even if you omit call to super in your constructor, super will still be called implicitly (super constructor without arguments, if defined in parent class, otherwise you'll receive Java compile error). So technically this super() is redundant, but I agree that sometimes explicit is better than implicit. Line 42: random = SecureRandom.getInstance("SHA1PRNG", "SUN"); //$NON-NLS-1$ $NON-NLS-2$ Line 43: //Call nextBytes this once to initialize the random implementation. Line 44: //We ignore the result, we just called it to initialize the implementation. Line 45: byte[] b = new byte[TOKEN_SIZE]; -- To view, visit http://gerrit.ovirt.org/31089 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic028b0d1f8a6fd0cf67863af51d02d892d33f5fb 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
