Hi Max, Thanks for your time and review.
Test refactorings look good to me :-) In regard to pkcs11.txt, we are currently using the cfg file to store configuration information (as before). I suggest to keep using it to leverage on previous work. The only change we are actually doing to configuration information is the "sql:/" prefix on the db path (to indicate a sqllite db). Kind regards, Martin.- On Mon, Dec 11, 2017 at 11:40 AM, Weijun Wang <weijun.w...@oracle.com> wrote: > Hi Martin > > Your src change looks fine, and if you think my test update is good, I can > push the changeset. > > Still, I need one confirmation. The modutil man page has "modutil supports > two types of databases: the legacy security > databases (cert8.db, key3.db, and secmod.db) and new SQLite databases > (cert9.db, key4.db, and pkcs11.txt)". So it looks like the pkcs11.txt file > is optional? > > Thanks > Max > > > > On Dec 11, 2017, at 11:16 AM, Weijun Wang <weijun.w...@oracle.com> > wrote: > > > > > > > >> On Dec 8, 2017, at 4:55 PM, Weijun Wang <weijun.w...@oracle.com> wrote: > >> > >> Hi Martin > >> > >> I've made some change and post a new webrev at > >> > >> http://cr.openjdk.java.net/~weijun/8165996/webrev.00/ > > > > More change in the same URL. > > > > - key4.db and cert9.db are saved in Secmod. > > > > - I modified the existing PKCS11Test/SecmodTest to load sqlite based dbs. > > > > Thanks > > Max > > > >> > >> The src part is unchanged. Major changes to test are: > >> > >> 1. PKCS11Test.getNSSLibDir() is used to get the NSS lib dir. Honestly > this is my 1st time touching NSS so hopefully it's not wrong. > >> > >> 2. I didn't used your private key and certs. Instead, an internal class > CertAndKeyGen is used. > >> > >> 3. I've saved "key4.db" and "cert9.db" as real files inside nss/sqlite. > I know binary files are extremely unwelcome in an open source project, but > maybe this time this is acceptable. We already have nss/db and nss/sqlite > is certainly not worse, and maybe we can write more test using this backend > later. > >> > >> 4. I also moved "nssdbsqlite" from /tmp to the current working > directory. For jtreg, cwd is always empty and will be cleaned/retained > after a test run. More importantly, no two test runs will use the same cwd. > >> > >> So nothing really changed. I still need to read about sql:/ to > understand the src fix. > >> > >> Thanks > >> Max > >> > >>> On Dec 8, 2017, at 2:33 PM, Weijun Wang <weijun.w...@oracle.com> > wrote: > >>> > >>> Hi Martin > >>> > >>> I'm just starting to read this patch. Two questions: > >>> > >>> 1. Is there a webpage on configDir using sql:/? > >>> > >>> 2. Your test hardcoded nssLibraryDirectory to be "/lib64". It would > need to be changed to either those inclosed the repository (macOS and > Windows) or in the system (others). Is there a version requirement? > >>> > >>> 3. The test contains a lot of binary data. Can you describe more > clearly on which is from where? Especially key4Content and cert9Content? In > fact, can they be recreated from the existing file based db inside > test/jdk/sun/security/pkcs11/nss/db? If yes, the test will be much > shorter. Please at least use multiple lines for the 2 keys. > >>> > >>> Thanks > >>> Max > >>> > >>>> On Nov 29, 2017, at 10:11 PM, Martin Balao <mba...@redhat.com> wrote: > >>>> > >>>> Hi, > >>>> > >>>> I'd like to propose a fix for JDK-8165996 - PKCS11 using NSS throws > an error regarding secmod.db when NSS uses sqlite [1]. > >>>> > >>>> Webrev01: > >>>> > >>>> * http://cr.openjdk.java.net/~akasko/mbalao/8165996.webrev.01/ > (browse online) > >>>> * http://cr.openjdk.java.net/~akasko/mbalao/8165996.webrev.01.zip > (download) > >>>> > >>>> Kind regards, > >>>> Martin.- > >>>> > >>>> -- > >>>> [1] - https://bugs.openjdk.java.net/browse/JDK-8165996 > >>> > >> > > > >
