You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ant.apache.org by Stefan Bodewig <bo...@apache.org> on 2017/04/19 12:58:35 UTC
Re: [32/34] ant git commit: java 5-8
Matt,
something I overlooked on my first pass
On 2017-04-13, <mb...@apache.org> wrote:
> http://git-wip-us.apache.org/repos/asf/ant/blob/b7d1e9bd/src/main/org/apache/tools/ant/helper/ProjectHelper2.java
> @@ -256,7 +259,7 @@ public class ProjectHelper2 extends ProjectHelper {
> zf = new ZipFile(org.apache.tools.ant.launch.Locator
> .fromJarURI(uri), "UTF-8");
> inputStream =
> - zf.getInputStream(zf.getEntry(uri.substring(pling + 1)));
> + zf.getInputStream(zf.getEntry(uri.substring(pling + 2)));
why did you change the offset from 1 to 2? Doesn't look right to me but
if it is we should probably change it for 1.9.x as well.
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: [32/34] ant git commit: java 5-8
Posted by Stefan Bodewig <bo...@apache.org>.
On 2017-04-20, Matt Benson wrote:
> On Thu, Apr 20, 2017 at 2:48 AM, Stefan Bodewig <bo...@apache.org> wrote:
>> Wouldn't we end up with a null InputStream and an exception being thrown
>> later on?
> In my debugging the XML parser allowed the null InputStream and took it
> upon itself to go find the named resource.
Ah, because of the SYSTEM id, I see.
>> so the current code is certainly wrong. I'll try to dig deeper into this
>> the coming days unless anybody else beats me to it.
> By "current" do you mean the code that increments past the ':' but not
> the '/', i.e. until I fixed said bug?
Right, the code in master has been fixed but the one in 1.9.x is still
broken. Apart from porting your fix I'd like to grep through the code
base to look for similar errors and maybe try to come up with a
test. Given the XML parser may be doing the job correctly anyway,
writing a test may not be possible.
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: [32/34] ant git commit: java 5-8
Posted by Matt Benson <gu...@gmail.com>.
On Thu, Apr 20, 2017 at 2:48 AM, Stefan Bodewig <bo...@apache.org> wrote:
> On 2017-04-19, Matt Benson wrote:
>
> > I had forgotten about this. When testing my other changes I noted that no
> > InputStream was returned here, due to the requested entry name including
> > the leading slash from the jar resource URL; the jar entries encountered
> > during testing, at least, lacked the leading slash.
>
> So this has never worked and nobody has noticed so far, strange.
>
> > Due to this the actual reading of the resource was deferred to the XML
> > parser. Skipping the slash fixed this issue for me and seemed
> > anecdotally to decrease the amount of time needed to run the
> > tests.
>
> Wouldn't we end up with a null InputStream and an exception being thrown
> later on?
>
> In my debugging the XML parser allowed the null InputStream and took it
upon itself to go find the named resource. This caused a weird exception
that somehow coincided with the changes to Path and Union to use the Java 8
Stream APIs, prompting me to find and fix this bug.
> > You of course know much more about the file format than I; I have been
> > unable to find a definitive resource that confirms that jar file
> > entries should always lack a leading separator.
>
> A leading slash would violate the ZIP spec
>
> https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT
>
> 4.4.17.1 The name of the file, with optional relative path.
> The path stored MUST not contain a drive or
> device letter, or a leading slash. ...
>
> so the current code is certainly wrong. I'll try to dig deeper into this
> the coming days unless anybody else beats me to it.
>
> By "current" do you mean the code that increments past the ':' but not the
'/', i.e. until I fixed said bug?
> Many thanks
>
> Not at all, thank you!
Matt
> Stefan
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> For additional commands, e-mail: dev-help@ant.apache.org
>
>
Re: [32/34] ant git commit: java 5-8
Posted by Stefan Bodewig <bo...@apache.org>.
On 2017-04-19, Matt Benson wrote:
> I had forgotten about this. When testing my other changes I noted that no
> InputStream was returned here, due to the requested entry name including
> the leading slash from the jar resource URL; the jar entries encountered
> during testing, at least, lacked the leading slash.
So this has never worked and nobody has noticed so far, strange.
> Due to this the actual reading of the resource was deferred to the XML
> parser. Skipping the slash fixed this issue for me and seemed
> anecdotally to decrease the amount of time needed to run the
> tests.
Wouldn't we end up with a null InputStream and an exception being thrown
later on?
> You of course know much more about the file format than I; I have been
> unable to find a definitive resource that confirms that jar file
> entries should always lack a leading separator.
A leading slash would violate the ZIP spec
https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT
4.4.17.1 The name of the file, with optional relative path.
The path stored MUST not contain a drive or
device letter, or a leading slash. ...
so the current code is certainly wrong. I'll try to dig deeper into this
the coming days unless anybody else beats me to it.
Many thanks
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: [32/34] ant git commit: java 5-8
Posted by Matt Benson <gu...@gmail.com>.
Hi Stefan,
I had forgotten about this. When testing my other changes I noted that no
InputStream was returned here, due to the requested entry name including
the leading slash from the jar resource URL; the jar entries encountered
during testing, at least, lacked the leading slash. Due to this the actual
reading of the resource was deferred to the XML parser. Skipping the slash
fixed this issue for me and seemed anecdotally to decrease the amount of
time needed to run the tests. You of course know much more about the file
format than I; I have been unable to find a definitive resource that
confirms that jar file entries should always lack a leading separator.
Matt
On Wed, Apr 19, 2017 at 7:58 AM, Stefan Bodewig <bo...@apache.org> wrote:
> Matt,
>
> something I overlooked on my first pass
>
> On 2017-04-13, <mb...@apache.org> wrote:
>
> > http://git-wip-us.apache.org/repos/asf/ant/blob/b7d1e9bd/
> src/main/org/apache/tools/ant/helper/ProjectHelper2.java
> > @@ -256,7 +259,7 @@ public class ProjectHelper2 extends ProjectHelper {
> > zf = new ZipFile(org.apache.tools.ant.
> launch.Locator
> > .fromJarURI(uri), "UTF-8");
> > inputStream =
> > - zf.getInputStream(zf.getEntry(uri.substring(pling
> + 1)));
> > + zf.getInputStream(zf.getEntry(uri.substring(pling
> + 2)));
>
> why did you change the offset from 1 to 2? Doesn't look right to me but
> if it is we should probably change it for 1.9.x as well.
>
> Stefan
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> For additional commands, e-mail: dev-help@ant.apache.org
>
>