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

[Jiri Vanek]

On 03/28/2012 02:41 PM, Thomas Meyer wrote:
> # HG changeset patch
> # User Thomas Meyer<thomas at m3y3r.de>
> # Date 1332937976 -7200
> # Node ID f6540088f06f2d9962e1c5b7858c4212f045759e
> # Parent  093896b370d3ed3f1fc3527084133b8e388bf0ae
> Fix IndexOutOfBoundException because of corrupt "recently_used" index file of cached files. Better solution would be to do this validation in CacheLRUWrapper.load()
>
> diff -r 093896b370d3 -r f6540088f06f netx/net/sourceforge/jnlp/cache/CacheUtil.java
> --- a/netx/net/sourceforge/jnlp/cache/CacheUtil.java	Wed Mar 28 12:08:10 2012 +0200
> +++ b/netx/net/sourceforge/jnlp/cache/CacheUtil.java	Wed Mar 28 14:32:56 2012 +0200
> @@ -354,17 +354,22 @@
>
>                   }
>               } while (entries == null);
> +
>               // Start searching from the most recent to least recent.
>               for (Entry<String, String>  e : entries) {
>                   final String key = e.getKey();
> -                final String path = e.getValue();
> +                String path = e.getValue();
>
>                   if (path != null) {
> -                    if (pathToURLPath(path).equals(urlPath.getPath())) { // Match found.
> -                        cacheFile = new File(path);
> -                        lruHandler.updateEntry(key);
> -                        break; // Stop searching since we got newest one already.
> -                    }
> +                    path=pathToURLPath(path);
> +                    if(path == null)
> +                        lruHandler.removeEntry(key);
> +                    else
> +                        if (path.equals(urlPath.getPath())) { // Match found.
> +                            cacheFile = new File(path);
> +                            lruHandler.updateEntry(key);
> +                            break; // Stop searching since we got newest one already.
> +                        }
>                   }
>               }
>               return cacheFile;
> @@ -377,6 +382,13 @@
>       private static String pathToURLPath(String path) {
>           int len = cacheDir.length();
>           int index = path.indexOf(File.separatorChar, len + 1);
> +
> +        /*
> +         * somehow the lru entries got corrupt. ignore this entry and
> +         * don't fail with an IndexOutOfBoundsException
> +         */
> +        if(index<  0)
> +            return null;
>           return path.substring(index);
>       }
>
>
>

Hi there! What are the prerequisites leading to do this  state?
I have fixed lru cache once already (for throwing NonIntegerValue exception) but I have never seen 
this error even when I was trying to corrupt cache, and I agree with you that this should be done in 
CacheLRUWrapper. Can you please provide (plaintext is enough) some reproducer?
If you will be willing to add this behaviou to 
icedtea-web/tests/jnlp_tests/signed/CacheReproducer/testcases/CacheReproducerTest.java it will be 
really great (You should do this according to commit policy O:) "IcedTea-Web code changes/new 
feature should be accompanied with appropriate tests (JUnit class and/or reproducer). If no tests 
are added/modified, changes should be accompanied with an explanation as to why. " O:)

 From cache record eg.:
1332779007444,35=/home/user/.icedtea/cache/35/http/localhost/SetContextClassLoader.jnlp

I see that this is removing "/home/user/.icedtea/cache/" correct?

So it can fail when:
  - recentlyused file have moved
  - javaws was killed during writeing

In both cases I would prefer cleaning of cache instead of neverending removing of entry.

So after "lruHandler.removeEntry(key);" should be save() at least to prevent this to happen again 
and again. But this brings other issues (like touching cache of other running javaws)

However, As best for fix this issue I'm considering change in CacheLRUWrapper's 
getLRUSortedEntries() to put to value already prepared path (or object with both prepared/full paths).
Then IndexOutOfBounds will be wrapped by LruCache exception and cache will be properly cleared.
However the cleanCache() method in CacheUtil will need small fixing after this too.

What do you think?


Thank you in advance, and Tyvm for getting the issue!
J.