Fwd: Re: [rfc][icedtea-web] fix cache in case of urls with query

[Jiri Vanek]

Just look nicer:)


-------- Forwarded Message --------
Subject: Re: [rfc][icedtea-web] fix cache in case of urls with query
Date: Wed, 27 May 2015 10:38:04 -0400 (EDT)
From: Jie Kang <jkang at redhat.com>
To: Jiri Vanek <jvanek at redhat.com>



----- Original Message -----
> On 05/26/2015 02:53 PM, Jiri Vanek wrote:
>
> Slightly imporoved version (+promissed tests). Ok to go?

Hello,

+            path.append(".").append(location.getQuery());

What is the append(".") for? Does it not work without it?

Everything else looks fine.



Regards,

>
> J.
> > On 05/26/2015 02:47 PM, Jie Kang wrote:
> >> Hello,
> >>
> >
> > Thanx for double check!
> >
> >> After the discussion over irc, personally, I would like a patch that
> >> includes a reproducer for the
> >> issue.
> >
> > You know taht reprodcuer is not simple to be add, because our internal
> > server donot suport this. But
> > I agree with need of it and will try to figure something out (as separate
> > chengeset)
> >  > I am okay with the fix as it is at the moment. Also, please remember to
> >  > verify unit tests pass.
> > This change causes CacheUtil.testUrlToPath to fail. In that regard, some
> > more unit tests to cover
> > the various inputs would be nice as well.
> >
> > Yes. I will. I will also add some unittsets testing exactly this issue.
> >
> > Thanx!
> >
> > J.
> >>
> >>
> >> Regards,
> >>
> >> ----- Original Message -----
> >>>
> >>>
> >>> ----- Original Message -----
> >>>> Another bug, originally related to -html, but much more widespread is
> >>>> related
> >>>> to compressed streams.
> >>>> Our glorious cache, is removing query when it is crating path to cached
> >>>> file.
> >>>> Obviously it is very
> >>>> wrong when jnlp file is based on this query, but mostly it is not
> >>>> observable.
> >>>>
> >>>> But, when this url with query is send as compressed stream, then .gz (or
> >>>> any
> >>>> other suffix depending
> >>>> on compression) is attached.  But.... it is attached *behind* query so
> >>>> once
> >>>> query is remove, the
> >>>> cache file have same suffix as final file, so it successfully rewrite
> >>>> itself.
> >>>>
> >>>> http://server/pahe.php?key=value
> >>>>
> >>>> => came as gzipped, so it is understand by itw as
> >>>> http://server/pahe.php?key=value.gz
> >>>>
> >>>>
> >>>> but both http://server/pahe.php?key=value and
> >>>> http://server/pahe.php?key=value.gz
> >>>> are stored in cache as
> >>>> http/server/pahe.php
> >>>>
> >>>>
> >>>> I Have chosen the fix by using also query in resulted name, but I have
> >>>> some
> >>>> feelings, that query was
> >>>> already removed in past - but
> >>>> http://icedtea.classpath.org/hg/icedtea-web/annotate/2b992e566c1f/netx/net/sourceforge/jnlp/cache/CacheUtil.java#l483
> >>>>
> >>>> speak differently, so...
> >>>>
> >>>> Another fix may be to put suffixes  after file, but before query. It is
> >>>> less
> >>>> invasive, and is fixing
> >>>> the bug I  was facing. But what about generated jnlps? I think he best
> >>>> way
> >>>> is
> >>>> ti use also whole
> >>>> query,,,, The filename is sanitized anyway.
> >>>
> >>> Hello,
> >>>
> >>> I prefer placing the suffixes after the file and before the query mainly
> >>> because it feels cleaner and we don't need to think about the
> >>> consequences
> >>> of saving the query.
> >>>
> >>> Can you explain a little more about what you mean by 'generated jnlps'?
> >>> If
> >>> it's too difficult to place suffixes before query, this fix is okay too.
> >>>
> >>> Is there a reproducer for this? If not please consider adding one so we
> >>> have
> >>> a regression check for this issue.
> >>>
> >>>
> >>> Regards,
> >>>
> >>>
> >>>
> >>>>
> >>>>
> >>>> J.
> >>>>
> >>>>
> >>>
> >>> --
> >>>
> >>> Jie Kang
> >>>
> >>> Java Team - Software Engineering Intern
> >>>
> >>
> >
>
>

-- 

Jie Kang

Java Team - Software Engineering Intern


-------------- next part --------------
A non-text attachment was scrubbed...
Name: toBePushed.patch
Type: text/x-patch
Size: 3551 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20150527/b4e14df6/toBePushed-0001.patch>