[rfc][icedtea-web] do not re-download href if environment is offline

[Jie Kang]

----- Original Message -----
> On 11/24/2014 04:40 PM, Jie Kang wrote:
> >
> >
> > ----- Original Message -----
> >> The whole offline environment is already in 1.5. Ok for head nd 1.5 too?
> >
> > Hello,
> >
> > I am not sure what the intentions of this change are.
> >
> > This changes the behaviour of the function. Is this intended?
> >
> > fromUrl(...):
> >              [...]
> >              if (!isLocal && haveHref){
> >                  //this is case when remote file have href to different
> >                  file
> >                  if (!location.equals(file.getSourceLocation())){
> >                      //mark local true, so the folowing condition will be
> >                      true and
> >                      //new jnlp file will be downlaoded
> >                      isLocal = true;
> >                      //maybe this check is to strict, and will force
> >                      redownlaod to often
> >                      //another check can be just on jnlp name. But it will
> >                      not work
> >                      //if the href will be the file of same name, but on
> >                      diferent path (or even domain)
> >                  }
> >              }
> >              if (isLocal && haveHref) {
> >                  file = new JNLPFile(file.getSourceLocation(),
> >                  parserSettings);
> >              }
> >              [...]
> >
> > For the case: (!isLocal && haveHref) : it checks for
> > (!location.equals(file.getSourceLocation()), which if true (not equal),
> > sets isLocal to true, in order to use the JNLP file located at
> > file.getSourceLocation() by taking advantage of the next if-statement
> > (isLocal && haveHref).
> >
> >
> > If you add JNLPRuntime.isOnline() check to the second if-statement, then
> > the above code will have new behaviour when offline: itt will not try to
> > load the JNLP file located at file.getSourceLocation() anymore.
> >
> >
> > I think a more correct fix would be:
> >
> > fromUrl(...):
> >              [...]
> >              if (!isLocal && haveHref &&
> >              !location.equals(file.getSourceLocation()){
> >                  file = new JNLPFile(file.getSourceLocation(),
> >                  parserSettings);
> >              }
> >              if (isLocal && haveHref && JNLPRuntime.isOnline()) {
> >                  file = new JNLPFile(file.getSourceLocation(),
> >                  parserSettings);
> >              }
> >              [...]
> >
> >
> >
> >
> > Also, if you look at the constructor for JNLPFile: new JNLPFile(url,
> > parsersettings), the code:
> >
> >              if (file.getSourceLocation() != null) {
> >                  haveHref = true;
> >              }
> >
> > is redundant, as file.getSourceLocation is never null. This isn't as big of
> > an issue though.
> >
> >
> >
> >
> > Regards,
> >
> >
> 
> Well, yes, the issue I'm heving is  NPE in SecurityDesc line (+-413 ) :
>              final URI codebase = file.getCodeBase().toURI().normalize();
> 
> Issue is that hreffed codebase may be null.  It is ok when file is online,
> but wrong  when we are
> going offline.
> 
> /me testing this patch2
> 
> 
> So yes, my inital patch was bad. The file should be "downloaded" (even only
> from cache) always.

Hello,


I think the patch is okay with just one minor nits:

+                JNLPFile hreffedFile = new JNLPFile(file.getSourceLocation(), parserSettings);

I would rename hreffedFile to fileFromHref.


You could also rename getSourceLocation() to getHrefAttribute() or something more obvious (what is source location?), but this is not 100% necessary, esp. for backport to 1.5.2.


Regards,

> 
> J.
> 
> 
> 
> 

-- 

Jie Kang