[RFC] Fix IndexOutOfBoundException because of corrupted entry in recently_used file

[Jiri Vanek]

Thanx for test! Logic itself is good, but there are two mistakes which causes the test to be not-working.
See notes inline for them.

Do you already have commit rights? If no, do you want them? If yes - can you please contact Mark Wielaard <mjw at redhat.com>? He will ask you for your public key and grant you all necessary permissions for pushing.

Also can you please always attach patch as atatchment? Eml body is always somehow corrupted before it flows through network and can cause unnecessary errors between  keyboard and computer on that (my ;o) ) end of line.

On 04/03/2012 03:14 PM, Thomas Meyer wrote:
> Here you go:
>
> # HG changeset patch
> # User Thomas Meyer<thomas at m3y3r.de>
> # Date 1333458674 -7200
> # Node ID a195e919572bce5f585f0fa1b595540519920a63
> # Parent  f6540088f06f2d9962e1c5b7858c4212f045759e
> Added Reproducer for corrupted cache entry file, containing an invalid path
>
> diff -r f6540088f06f -r a195e919572b Makefile.am
> --- a/Makefile.am	Wed Mar 28 14:32:56 2012 +0200
> +++ b/Makefile.am	Tue Apr 03 15:11:14 2012 +0200
> @@ -565,7 +565,7 @@
>   	cd $(JNLP_TESTS_ENGINE_DIR) ; \
>   	class_names=`cat $(REPRODUCERS_CLASS_NAMES)` ; \
>   	CLASSPATH=$(NETX_DIR)/lib/classes.jar:$(JUNIT_JAR):$(JUNIT_RUNNER_JAR):. \
> -	  $(BOOT_DIR)/bin/java -Dtest.server.dir=$(JNLP_TESTS_SERVER_DEPLOYDIR) -Djavaws.build.bin=$(DESTDIR)$(bindir)/javaws \
> +	  $(BOOT_DIR)/bin/java -Dtest.server.dir=$(JNLP_TESTS_SERVER_DEPLOYDIR) -Djavaws.build.bin=$(DESTDIR)$(bindir)/$(javaws) \
Thanx for catch!
>   	 -Xbootclasspath:$(RUNTIME) CommandLine $$class_names \
>   	>  stdout.log 2>  stderr.log ; \
>   	 cat stdout.log ; \
> diff -r f6540088f06f -r a195e919572b tests/jnlp_tests/signed/CacheReproducer/testcases/CacheReproducerTest.java
> --- a/tests/jnlp_tests/signed/CacheReproducer/testcases/CacheReproducerTest.java	Wed Mar 28 14:32:56 2012 +0200
> +++ b/tests/jnlp_tests/signed/CacheReproducer/testcases/CacheReproducerTest.java	Tue Apr 03 15:11:14 2012 +0200
> @@ -148,7 +148,7 @@
>
>       @Test
>       public void coruptAndRunCache2() throws Exception {
> -         clearAndEvaluateCache();
> +        clearAndEvaluateCache();
>           evaluateSimpleTest1OkCache(runSimpleTest1());
>           assertCacheIsNotEmpty();
>           breakCache1();
> @@ -164,6 +164,24 @@
>           assertLruExceptionNOTappeared(pr2);
>       }
>
Can I ask for javadoc? I admit there is missing javadoc in rest of the testfile, but until now it was doing just one type of corruption.
> +    @Test
> +    public void coruptAndRunCache3() throws Exception {
> +        clearAndEvaluateCache();
> +        evaluateSimpleTest1OkCache(runSimpleTest1());
> +        assertCacheIsNotEmpty();
> +        breakCache3();
> +        ProcessResult pr = runSimpleTest1();
> +        assertLruExceptionAppeared(pr);
This is not correct right now. This should test for IndexOutOfBoundsException.
My opinion is to check for IndexOutOfBoundsException have not appeard and on next line assertLruExceptionAppeared - considering that your fix will print out LruException somewhere (which it should as we have agreed)
The rest of the test should be ok under same circumstances (mostly that cache will be fixed as is leading from those circumstances)
> +        evaluateSimpleTest1OkCache(pr);
> +        ProcessResult pr3 = runSimpleTest1();
> +        evaluateSimpleTest1OkCache(pr3);
> +        assertLruExceptionNOTappeared(pr3);
> +        clearAndEvaluateCache();
> +        ProcessResult pr2 = runSimpleTest1();
> +        evaluateSimpleTest1OkCache(pr2);
> +        assertLruExceptionNOTappeared(pr2);
> +    }
> +
>       private void assertLruExceptionNOTappeared(ProcessResult pr2) {
>           Assert.assertFalse("serr should NOT contain " + lre, pr2.stderr.contains(lre));
>       }
> @@ -174,7 +192,7 @@
>
>       @Test
>       public void coruptAndRunCache1Signed() throws Exception {
> -         clearAndEvaluateCache();
> +        clearAndEvaluateCache();
>           evaluateSimpleTest1OkCache(runSimpleTest1());
>           assertCacheIsNotEmpty();
>           breakCache1();
> @@ -189,7 +207,7 @@
>
>       @Test
>       public void coruptAndRunCache2Signed() throws Exception {
> -         clearAndEvaluateCache();
> +        clearAndEvaluateCache();
>           evaluateSimpleTest1OkCache(runSimpleTest1());
>           assertCacheIsNotEmpty();
>           breakCache1();
> @@ -236,7 +254,7 @@
>       }
>
>
> -    //next four tests are designed to ensure, that corupted cache wil not break already loaded cached files
> +    //next four tests are designed to ensure, that corrupted cache will not break already loaded cached files
>       public static final String CR1 = "CacheReproducer1";
>       public static final String CR2 = "CacheReproducer2";
>       public static final String CR11 = "CacheReproducer1_1";
> @@ -378,6 +396,14 @@
>           return s;
>       }
>

I'm afraid this will not work as expected. After this change the file looks like:
#netx file
#Tue Apr 03 16:26:23 CEST 2012
1333463182549,1=/home/jvanek/.icedtea/cache/1/http/localhost/simpletest1.jar
1333463182202,0=/ho

And because launched example is simpletest1, then it will be always found and the corrupted line will never be reached.
My advice here is to corrupt all lines and so be sure :)

Also maybe small test similar to assertBreakersAreWorking can be worthy, but I do not insists.
> +    private static String breakLastPath(String s) {
> +        int i = s.lastIndexOf(home);
> +        if(i>= 0&&  i<= s.length()&&  home.length()>  4)
> +            return s.substring(0, i + 3);
> +
> +        return null;
> +    }
> +
>       private static void breakCache1() throws IOException {
>           String s = loadCacheFile();
>           s = breakAll(s);
> @@ -390,6 +416,12 @@
>           ServerAccess.saveFile(s, icedteaCacheFile);
>       }
>
> +    private static void breakCache3() throws IOException {
> +        String s = loadCacheFile();
> +        s = breakLastPath(s);
> +        ServerAccess.saveFile(s, icedteaCacheFile);
> +    }
> +
>       private static ServerAccess.ProcessResult runSimpleTest1() throws Exception {
>           return runSimpleTest1(null, "simpletest1");
>       }
>
>

Best regards and thanx for effort,
   J.