From: Felix Dörre Date: Sat, 24 Oct 2015 13:03:52 +0000 (+0200) Subject: UPD: minor consistency cleanups X-Git-Url: https://code.wpia.club/?p=gigi.git;a=commitdiff_plain;h=b47144d6f3bf6b6eb1ec477de9d2af38256f074f UPD: minor consistency cleanups --- diff --git a/src/org/cacert/gigi/dbObjects/Assurance.java b/src/org/cacert/gigi/dbObjects/Assurance.java index 2d7c5934..cb0bcc49 100644 --- a/src/org/cacert/gigi/dbObjects/Assurance.java +++ b/src/org/cacert/gigi/dbObjects/Assurance.java @@ -1,7 +1,9 @@ package org.cacert.gigi.dbObjects; import org.cacert.gigi.database.GigiResultSet; +import org.cacert.gigi.dbObjects.wrappers.DataContainer; +@DataContainer public class Assurance { public enum AssuranceType { diff --git a/src/org/cacert/gigi/dbObjects/Certificate.java b/src/org/cacert/gigi/dbObjects/Certificate.java index 4d672572..4300ac9a 100644 --- a/src/org/cacert/gigi/dbObjects/Certificate.java +++ b/src/org/cacert/gigi/dbObjects/Certificate.java @@ -23,7 +23,7 @@ import org.cacert.gigi.database.GigiResultSet; import org.cacert.gigi.util.KeyStorage; import org.cacert.gigi.util.Notary; -public class Certificate { +public class Certificate implements IdCachable { public enum SANType { EMAIL("email"), DNS("DNS"); @@ -133,7 +133,7 @@ public class Certificate { private CACertificate ca; - public Certificate(User owner, HashMap dn, String md, String csr, CSRType csrType, CertificateProfile profile, SubjectAlternateName... sans) throws GigiApiException { + public Certificate(User owner, HashMap dn, String md, String csr, CSRType csrType, CertificateProfile profile, SubjectAlternateName... sans) throws GigiApiException, IOException { if ( !profile.canBeIssuedBy(owner)) { throw new GigiApiException("You are not allowed to issue these certificates."); } @@ -148,6 +148,44 @@ public class Certificate { this.csrType = csrType; this.profile = profile; this.sans = Arrays.asList(sans); + synchronized (Certificate.class) { + + GigiPreparedStatement inserter = DatabaseConnection.getInstance().prepare("INSERT INTO certs SET md=?::`mdType`, csr_type=?::`csrType`, crt_name='', memid=?, profile=?"); + inserter.setString(1, md.toLowerCase()); + inserter.setString(2, csrType.toString()); + inserter.setInt(3, owner.getId()); + inserter.setInt(4, profile.getId()); + inserter.execute(); + id = inserter.lastInsertId(); + + GigiPreparedStatement san = DatabaseConnection.getInstance().prepare("INSERT INTO `subjectAlternativeNames` SET `certId`=?, contents=?, type=?::`SANType`"); + for (SubjectAlternateName subjectAlternateName : sans) { + san.setInt(1, id); + san.setString(2, subjectAlternateName.getName()); + san.setString(3, subjectAlternateName.getType().getOpensslName()); + san.execute(); + } + + GigiPreparedStatement insertAVA = DatabaseConnection.getInstance().prepare("INSERT INTO `certAvas` SET `certId`=?, name=?, value=?"); + insertAVA.setInt(1, id); + for (Entry e : dn.entrySet()) { + insertAVA.setString(2, e.getKey()); + insertAVA.setString(3, e.getValue()); + insertAVA.execute(); + } + File csrFile = KeyStorage.locateCsr(id); + csrName = csrFile.getPath(); + try (FileOutputStream fos = new FileOutputStream(csrFile)) { + fos.write(csr.getBytes("UTF-8")); + } + + GigiPreparedStatement updater = DatabaseConnection.getInstance().prepare("UPDATE `certs` SET `csr_name`=? WHERE id=?"); + updater.setString(1, csrName); + updater.setInt(2, id); + updater.execute(); + + cache.put(this); + } } private Certificate(GigiResultSet rs) { @@ -246,39 +284,6 @@ public class Certificate { } Notary.writeUserAgreement(owner, "CCA", "issue certificate", "", true, 0); - GigiPreparedStatement inserter = DatabaseConnection.getInstance().prepare("INSERT INTO certs SET md=?::`mdType`, csr_type=?::`csrType`, crt_name='', memid=?, profile=?"); - inserter.setString(1, md.toLowerCase()); - inserter.setString(2, csrType.toString()); - inserter.setInt(3, owner.getId()); - inserter.setInt(4, profile.getId()); - inserter.execute(); - id = inserter.lastInsertId(); - - GigiPreparedStatement san = DatabaseConnection.getInstance().prepare("INSERT INTO `subjectAlternativeNames` SET `certId`=?, contents=?, type=?::`SANType`"); - for (SubjectAlternateName subjectAlternateName : sans) { - san.setInt(1, id); - san.setString(2, subjectAlternateName.getName()); - san.setString(3, subjectAlternateName.getType().getOpensslName()); - san.execute(); - } - - GigiPreparedStatement insertAVA = DatabaseConnection.getInstance().prepare("INSERT INTO `certAvas` SET `certId`=?, name=?, value=?"); - insertAVA.setInt(1, id); - for (Entry e : dn.entrySet()) { - insertAVA.setString(2, e.getKey()); - insertAVA.setString(3, e.getValue()); - insertAVA.execute(); - } - File csrFile = KeyStorage.locateCsr(id); - csrName = csrFile.getPath(); - try (FileOutputStream fos = new FileOutputStream(csrFile)) { - fos.write(csr.getBytes("UTF-8")); - } - - GigiPreparedStatement updater = DatabaseConnection.getInstance().prepare("UPDATE `certs` SET `csr_name`=? WHERE id=?"); - updater.setString(1, csrName); - updater.setInt(2, id); - updater.execute(); return Job.sign(this, start, period); } @@ -352,33 +357,46 @@ public class Certificate { return profile; } - public static Certificate getBySerial(String serial) { + public synchronized static Certificate getBySerial(String serial) { if (serial == null || "".equals(serial)) { return null; } - // TODO caching? try { String concat = "string_agg(concat('/', `name`, '=', REPLACE(REPLACE(value, '\\\\', '\\\\\\\\'), '/', '\\\\/')), '')"; GigiPreparedStatement ps = DatabaseConnection.getInstance().prepare("SELECT certs.id, " + concat + " as `subject`, `md`, `csr_name`, `crt_name`,`memid`, `profile`, `certs`.`serial` FROM `certs` LEFT JOIN `certAvas` ON `certAvas`.`certId`=`certs`.`id` WHERE `serial`=? GROUP BY `certs`.`id`"); ps.setString(1, serial); GigiResultSet rs = ps.executeQuery(); - return new Certificate(rs); + int id = rs.getInt(1); + Certificate c1 = cache.get(id); + if (c1 != null) { + return c1; + } + Certificate certificate = new Certificate(rs); + cache.put(certificate); + return certificate; } catch (IllegalArgumentException e) { } return null; } - public static Certificate getById(int id) { + private static ObjectCache cache = new ObjectCache<>(); + + public synchronized static Certificate getById(int id) { + Certificate cacheRes = cache.get(id); + if (cacheRes != null) { + return cacheRes; + } - // TODO caching? try { String concat = "string_agg(concat('/', `name`, '=', REPLACE(REPLACE(value, '\\\\', '\\\\\\\\'), '/', '\\\\/')), '')"; GigiPreparedStatement ps = DatabaseConnection.getInstance().prepare("SELECT certs.id, " + concat + " as subject, md, csr_name, crt_name,memid, profile, certs.serial FROM `certs` LEFT JOIN `certAvas` ON `certAvas`.`certId`=certs.id WHERE certs.id=? GROUP BY certs.id"); ps.setInt(1, id); GigiResultSet rs = ps.executeQuery(); - return new Certificate(rs); + Certificate c = new Certificate(rs); + cache.put(c); + return c; } catch (IllegalArgumentException e) { } diff --git a/src/org/cacert/gigi/dbObjects/Domain.java b/src/org/cacert/gigi/dbObjects/Domain.java index c165c4ab..6b0e8283 100644 --- a/src/org/cacert/gigi/dbObjects/Domain.java +++ b/src/org/cacert/gigi/dbObjects/Domain.java @@ -52,10 +52,12 @@ public class Domain implements IdCachable, Verifyable { } public Domain(User owner, String suffix) throws GigiApiException { - checkCertifyableDomain(suffix, owner.isInGroup(Group.CODESIGNING)); - this.owner = owner; - this.suffix = suffix; - + synchronized (Domain.class) { + checkCertifyableDomain(suffix, owner.isInGroup(Group.CODESIGNING)); + this.owner = owner; + this.suffix = suffix; + insert(); + } } public static void checkCertifyableDomain(String s, boolean hasPunycodeRight) throws GigiApiException { @@ -126,19 +128,17 @@ public class Domain implements IdCachable, Verifyable { } } - public void insert() throws GigiApiException { - synchronized (Domain.class) { - if (id != 0) { - throw new GigiApiException("already inserted."); - } - checkInsert(suffix); - GigiPreparedStatement ps = DatabaseConnection.getInstance().prepare("INSERT INTO `domains` SET memid=?, domain=?"); - ps.setInt(1, owner.getId()); - ps.setString(2, suffix); - ps.execute(); - id = ps.lastInsertId(); - myCache.put(this); + private void insert() throws GigiApiException { + if (id != 0) { + throw new GigiApiException("already inserted."); } + checkInsert(suffix); + GigiPreparedStatement ps = DatabaseConnection.getInstance().prepare("INSERT INTO `domains` SET memid=?, domain=?"); + ps.setInt(1, owner.getId()); + ps.setString(2, suffix); + ps.execute(); + id = ps.lastInsertId(); + myCache.put(this); } public void delete() throws GigiApiException { diff --git a/src/org/cacert/gigi/dbObjects/Job.java b/src/org/cacert/gigi/dbObjects/Job.java index 321b89d0..c02a0467 100644 --- a/src/org/cacert/gigi/dbObjects/Job.java +++ b/src/org/cacert/gigi/dbObjects/Job.java @@ -8,7 +8,7 @@ import org.cacert.gigi.database.GigiPreparedStatement; import org.cacert.gigi.database.GigiResultSet; import org.cacert.gigi.output.CertificateValiditySelector; -public class Job { +public class Job implements IdCachable { private int id; @@ -38,19 +38,19 @@ public class Job { ps.setDate(3, start); ps.setString(4, period); ps.execute(); - return new Job(ps.lastInsertId()); + return cache.put(new Job(ps.lastInsertId())); } - public static Job revoke(Certificate targetId) { + public synchronized static Job revoke(Certificate targetId) { GigiPreparedStatement ps = DatabaseConnection.getInstance().prepare("INSERT INTO `jobs` SET targetId=?, task=?::`jobType`"); ps.setInt(1, targetId.getId()); ps.setString(2, JobType.REVOKE.getName()); ps.execute(); - return new Job(ps.lastInsertId()); + return cache.put(new Job(ps.lastInsertId())); } - public boolean waitFor(int max) throws InterruptedException { + public synchronized boolean waitFor(int max) throws InterruptedException { long start = System.currentTimeMillis(); GigiPreparedStatement ps = DatabaseConnection.getInstance().prepare("SELECT 1 FROM `jobs` WHERE id=? AND state='open'"); ps.setInt(1, id); @@ -66,4 +66,28 @@ public class Job { rs.close(); return true; } + + @Override + public int getId() { + return id; + } + + static ObjectCache cache = new ObjectCache<>(); + + public synchronized static Job getById(int id) { + Job i = cache.get(id); + if (i != null) { + return i; + } + GigiPreparedStatement ps = DatabaseConnection.getInstance().prepare("SELECT 1 FROM `jobs` WHERE id=?'"); + ps.setInt(1, id); + GigiResultSet rs = ps.executeQuery(); + if (rs.next()) { + Job j = new Job(id); + cache.put(j); + return j; + } + return null; + + } } diff --git a/src/org/cacert/gigi/dbObjects/Name.java b/src/org/cacert/gigi/dbObjects/Name.java index f6046c91..d0cf00f7 100644 --- a/src/org/cacert/gigi/dbObjects/Name.java +++ b/src/org/cacert/gigi/dbObjects/Name.java @@ -3,10 +3,12 @@ package org.cacert.gigi.dbObjects; import java.io.PrintWriter; import java.util.Map; +import org.cacert.gigi.dbObjects.wrappers.DataContainer; import org.cacert.gigi.localisation.Language; import org.cacert.gigi.output.template.Outputable; import org.cacert.gigi.util.HTMLEncoder; +@DataContainer public class Name implements Outputable { private final String fname; diff --git a/src/org/cacert/gigi/dbObjects/ObjectCache.java b/src/org/cacert/gigi/dbObjects/ObjectCache.java index c17dcacf..0d9fc14b 100644 --- a/src/org/cacert/gigi/dbObjects/ObjectCache.java +++ b/src/org/cacert/gigi/dbObjects/ObjectCache.java @@ -14,8 +14,9 @@ public class ObjectCache { caches.add(this); } - public void put(T c) { + public T put(T c) { hashmap.put(c.getId(), new WeakReference(c)); + return c; } public T get(int id) { diff --git a/src/org/cacert/gigi/dbObjects/wrappers/DataContainer.java b/src/org/cacert/gigi/dbObjects/wrappers/DataContainer.java new file mode 100644 index 00000000..db14af60 --- /dev/null +++ b/src/org/cacert/gigi/dbObjects/wrappers/DataContainer.java @@ -0,0 +1,9 @@ +package org.cacert.gigi.dbObjects.wrappers; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Target; + +@Target(ElementType.TYPE) +public @interface DataContainer { + +} diff --git a/src/org/cacert/gigi/pages/account/certs/CertificateRequest.java b/src/org/cacert/gigi/pages/account/certs/CertificateRequest.java index 026354da..eb547cd3 100644 --- a/src/org/cacert/gigi/pages/account/certs/CertificateRequest.java +++ b/src/org/cacert/gigi/pages/account/certs/CertificateRequest.java @@ -445,8 +445,13 @@ public class CertificateRequest { if ( !error.isEmpty()) { throw error; } - return new Certificate(u, subject, selectedDigest.toString(), // - this.csr, this.csrType, profile, SANs.toArray(new SubjectAlternateName[SANs.size()])); + try { + return new Certificate(u, subject, selectedDigest.toString(), // + this.csr, this.csrType, profile, SANs.toArray(new SubjectAlternateName[SANs.size()])); + } catch (IOException e) { + e.printStackTrace(); + } + return null; } private String verifyName(GigiApiException error, PropertyTemplate nameTemp, PropertyTemplate wotUserTemp, String verifiedCN) { diff --git a/src/org/cacert/gigi/pages/account/domain/DomainAddForm.java b/src/org/cacert/gigi/pages/account/domain/DomainAddForm.java index 6244e9d6..5b1c76e1 100644 --- a/src/org/cacert/gigi/pages/account/domain/DomainAddForm.java +++ b/src/org/cacert/gigi/pages/account/domain/DomainAddForm.java @@ -36,7 +36,6 @@ public class DomainAddForm extends Form { throw new GigiApiException("No domain inserted."); } Domain d = new Domain(target, parameter); - d.insert(); pcf.setTarget(d); pcf.submit(out, req); return true; diff --git a/tests/org/cacert/gigi/TestDomain.java b/tests/org/cacert/gigi/TestDomain.java index bf7118df..5c242bef 100644 --- a/tests/org/cacert/gigi/TestDomain.java +++ b/tests/org/cacert/gigi/TestDomain.java @@ -21,7 +21,6 @@ public class TestDomain extends ManagedTest { assertEquals(0, us.getDomains().length); Domain d = new Domain(us, "v1example.org"); assertEquals(0, d.getId()); - d.insert(); Domain[] domains = us.getDomains(); assertEquals(1, domains.length); assertEquals("v1example.org", domains[0].getSuffix()); @@ -32,7 +31,6 @@ public class TestDomain extends ManagedTest { Domain d2 = new Domain(us, "v2-example.org"); assertEquals(0, d2.getId()); - d2.insert(); domains = us.getDomains(); assertEquals(2, domains.length); @@ -53,10 +51,8 @@ public class TestDomain extends ManagedTest { @Test public void testDoubleDomain() throws InterruptedException, GigiApiException { Domain d = new Domain(us, "dub-example.org"); - d.insert(); try { Domain d2 = new Domain(us, "dub-example.org"); - d2.insert(); fail("expected exception"); } catch (GigiApiException e) { // expected @@ -66,22 +62,8 @@ public class TestDomain extends ManagedTest { @Test public void testDoubleDomainDelete() throws InterruptedException, GigiApiException { Domain d = new Domain(us, "delexample.org"); - d.insert(); d.delete(); Domain d2 = new Domain(us, "delexample.org"); - d2.insert(); - } - - @Test - public void testDoubleInsertDomain() throws InterruptedException, GigiApiException { - Domain d = new Domain(us, "dins-example.org"); - d.insert(); - try { - d.insert(); - fail("expected exception"); - } catch (GigiApiException e) { - // expected - } } } diff --git a/tests/org/cacert/gigi/TestObjectCache.java b/tests/org/cacert/gigi/TestObjectCache.java index 6824c379..b1272a0f 100644 --- a/tests/org/cacert/gigi/TestObjectCache.java +++ b/tests/org/cacert/gigi/TestObjectCache.java @@ -35,7 +35,6 @@ public class TestObjectCache extends ManagedTest { @Test public void testDomainCache() throws GigiApiException { Domain d = new Domain(User.getById(uid), "example.org"); - d.insert(); assertThat(d, is(sameInstance(Domain.getById(d.getId())))); assertThat(Domain.getById(d.getId()), is(sameInstance(Domain.getById(d.getId())))); diff --git a/tests/org/cacert/gigi/TestUser.java b/tests/org/cacert/gigi/TestUser.java index f91ab113..edee51f0 100644 --- a/tests/org/cacert/gigi/TestUser.java +++ b/tests/org/cacert/gigi/TestUser.java @@ -65,9 +65,9 @@ public class TestUser extends ManagedTest { User u = User.getById(id); new EmailAddress(u, uq + "b@email.org", Locale.ENGLISH); new EmailAddress(u, uq + "c@email.org", Locale.ENGLISH); - new Domain(u, uq + "a-testdomain.org").insert(); - new Domain(u, uq + "b-testdomain.org").insert(); - new Domain(u, uq + "c-testdomain.org").insert(); + new Domain(u, uq + "a-testdomain.org"); + new Domain(u, uq + "b-testdomain.org"); + new Domain(u, uq + "c-testdomain.org"); assertEquals(3, u.getEmails().length); assertEquals(3, u.getDomains().length); assertTrue(u.isValidDomain(uq + "a-testdomain.org")); diff --git a/tests/org/cacert/gigi/pages/admin/TestSEAdminPageUserDomainSearch.java b/tests/org/cacert/gigi/pages/admin/TestSEAdminPageUserDomainSearch.java index 8921c3a8..3229908f 100644 --- a/tests/org/cacert/gigi/pages/admin/TestSEAdminPageUserDomainSearch.java +++ b/tests/org/cacert/gigi/pages/admin/TestSEAdminPageUserDomainSearch.java @@ -34,8 +34,7 @@ public class TestSEAdminPageUserDomainSearch extends ClientTest { int id = createVerifiedUser("Först", "Secönd", mail, TEST_PASSWORD); User user = User.getById(id); String domainName = createUniqueName() + ".org"; - Domain d = new Domain(user, domainName); - d.insert(); + new Domain(user, domainName); URLConnection uc = new URL("https://" + getServerName() + FindDomainPage.PATH).openConnection(); uc.addRequestProperty("Cookie", cookie); String csrf = getCSRF(uc, 0); @@ -57,7 +56,6 @@ public class TestSEAdminPageUserDomainSearch extends ClientTest { User user = User.getById(id); String domainName = createUniqueName() + ".org"; Domain d = new Domain(user, domainName); - d.insert(); URLConnection uc = new URL("https://" + getServerName() + FindDomainPage.PATH).openConnection(); uc.addRequestProperty("Cookie", cookie); String csrf = getCSRF(uc, 0); diff --git a/util-testing/org/cacert/gigi/pages/Manager.java b/util-testing/org/cacert/gigi/pages/Manager.java index 9b4144a6..8b81820b 100644 --- a/util-testing/org/cacert/gigi/pages/Manager.java +++ b/util-testing/org/cacert/gigi/pages/Manager.java @@ -53,7 +53,9 @@ public class Manager extends Page { f = EmailAddress.class.getDeclaredField("hash"); f.setAccessible(true); } catch (ReflectiveOperationException e) { - throw new Error(e); + // TODO + System.out.println("I don't have 'hash', we are working probably in layered mode. Test Manager may not work."); + // throw new Error(e); } } @@ -153,6 +155,10 @@ public class Manager extends Page { gc.set(1990, 0, 1); User u = new User(email, "xvXV12°§", new Name("Först", "Läst", "Müddle", "Süffix"), new Date(gc.getTime().getTime()), Locale.ENGLISH); EmailAddress ea = u.getEmails()[0]; + if (f == null) { + System.out.println("verification failed"); + return; + } String hash = (String) f.get(ea); ea.verify(hash); @@ -209,9 +215,13 @@ public class Manager extends Page { User u = User.getByEmail(req.getParameter("addEmailEmail")); try { EmailAddress ea = new EmailAddress(u, req.getParameter("addEmailNew"), Locale.ENGLISH); - String hash = (String) f.get(ea); - ea.verify(hash); - resp.getWriter().println("Email added and verified"); + if (f != null) { + String hash = (String) f.get(ea); + ea.verify(hash); + resp.getWriter().println("Email added and verified"); + } else { + resp.getWriter().println("Email added but verificatio failed."); + } } catch (IllegalArgumentException e) { e.printStackTrace(); resp.getWriter().println("An internal error occured.");