Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

archive entry has inconsistent/wrong leading slashes #217

Closed
stackoverflow opened this issue Aug 16, 2015 · 31 comments
Closed

archive entry has inconsistent/wrong leading slashes #217

stackoverflow opened this issue Aug 16, 2015 · 31 comments
Labels
Milestone

Comments

@stackoverflow
Copy link

In version 1.4 PermMapper was changed and now instead of creating a new TarArchiveEntry it uses the one provided to the map function. Before the TarArchiveEntry constructor was called with preserveLeadingSlashes true.
Now the setName method is called without the second parameter meaning preserveLeadingSlashes is false.
So, in version 1.3 if I use a PermMapper with prefix "/etc/default" my conffile entry looks like:
/etc/default/file-name
but in 1.4 it's generated as
etc/default/file-name

can you pass true as the second parameter to setName and fix this?

@tcurdt tcurdt added the bug label Aug 17, 2015
@tcurdt tcurdt added this to the 1.5 milestone Aug 17, 2015
@tcurdt
Copy link
Owner

tcurdt commented Dec 28, 2015

@tmortagne
Copy link
Collaborator

Yes only the TarArchiveEntry constructor takes a preserveLeadingSlashes parameter unfortunately. We should probably put back the re-creation of the TarArchiveEntry.

@tcurdt
Copy link
Owner

tcurdt commented Dec 30, 2015

Hm - but the producer creates the TarArchiveEntry and the default is to create it with preserveLeadingSlashes set to true. The mapper just passes this on.

I think we need a testcase for this.

@tmortagne
Copy link
Collaborator

bq. but the producer creates the TarArchiveEntry and the default is to create it with preserveLeadingSlashes set to true

The issue is not how the entry is created but how the name is set. The boolean is only used when setting the same in the constructor, it does not remember it. And setName behave as if you were calling the constructor with false basically.

@tmortagne
Copy link
Collaborator

There used to be a test because I had to struggle with the same issue and the test was modified by the same commit that caused this issue...

@tcurdt
Copy link
Owner

tcurdt commented Dec 31, 2015

The boolean is only used when setting the same in the constructor, it does not remember it. And setName behave as if you were calling the constructor with false basically.

Well, that is counter intuitive. Should we maybe fix this in commons compress then?

@tmortagne
Copy link
Collaborator

Well, that is counter intuitive. Should we maybe fix this in commons compress then?

Having a boolean in the setName would be the best probably.

@tcurdt
Copy link
Owner

tcurdt commented Dec 31, 2015

Maybe a little too much magic here https://github.com/apache/commons-compress/blob/master/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveEntry.java#L1072

but looking at official packages I am seeing ./usr/share/ - so no leading path at all.

@tcurdt
Copy link
Owner

tcurdt commented Jan 6, 2016

Either way I have opened https://issues.apache.org/jira/browse/COMPRESS-328

@tcurdt
Copy link
Owner

tcurdt commented Jan 16, 2016

I fixed the preserveLeadingSlashes support in Commons Compress. Will be in the next release.

@tcurdt
Copy link
Owner

tcurdt commented Jan 16, 2016

That said - I am quite sure the tar archive should be in fact all relative. We should check that.

@tcurdt tcurdt modified the milestones: 1.6, 1.5 Jan 16, 2016
@tcurdt tcurdt changed the title preserve leading slashes always false in conffile when using a PermMapper with prefix archive entry has inconsistent/wrong leading slashes Jan 16, 2016
@tcurdt
Copy link
Owner

tcurdt commented May 15, 2016

There was a new release of commons compress that helps fixing this.

@peternewman
Copy link
Contributor

It's probably worth clarifying (and something I probably lost in #169 before), but there's actually a lot of subtlety in the start of the path in the different files, compare these two examples:

md5sums:
5565b8b26108c49ba575ba452cd69b3e etc/ucf.conf

conffiles:
/etc/ucf.conf

So even if we can get the Tar sorted, we may need to manually tweak the output for different file usage anyway.

@tcurdt
Copy link
Owner

tcurdt commented Sep 7, 2016

From official packages...

md5sums:

f5959cba3613d49c4bbe79ebafffedd1  usr/sbin/update-exim4.conf
1dfaa4ea45d537bb1ac6963b282aba62  usr/sbin/update-exim4.conf.template
64b2f40b5c74fa75c7ec2297e1732939  usr/sbin/update-exim4defaults
1e7a03f823b39a7d08ca48e9794595a1  usr/share/bug/exim4-config/script
b1b1197e2342d4e3e5db744c961ee0f1  usr/share/doc/exim4-config/NEWS.Debian.gz

conffiles:

/etc/email-addresses
/etc/exim4/conf.d/acl/00_exim4-config_header
/etc/exim4/conf.d/acl/20_exim4-config_local_deny_exceptions
/etc/exim4/conf.d/acl/30_exim4-config_check_mail

So I would call it inconsistent but correct :)

@peternewman
Copy link
Contributor

Yes sorry, I think we're in agreement here. So does jdeb do it yet (and if so, do we need the latest Commons Compress too), given this issue is still open? It was more flagging up as I realised my previous comment wasn't too clear, I didn't want someone just swapping all the ./ for / and assuming it was fixed.

@tcurdt
Copy link
Owner

tcurdt commented Sep 9, 2016

The pom is still listing commons-compress 1.10 - but it was fixed in 1.11.
I guess it's time for fixing this in jdeb as well now.

@TomyLobo
Copy link

At least the conffiles portion of this bug still exists in 1.5

@tcurdt
Copy link
Owner

tcurdt commented Feb 25, 2017

@TomyLobo could you elaborate?
Is this still a problem with master?

@TomyLobo
Copy link

TomyLobo commented Feb 25, 2017

I don't have that particular project here right now and it has been a while since I encountered the problem, but iirc, in 1.5, files listed in "conffiles" (in the control section of the .deb package) still had no leading slashes if the list was automatically generated..

The command "dpkg-deb -I mypackage.deb conffiles" should show you the contents of the conffiles file, but again, i have no way to test it right now :)

@tcurdt
Copy link
Owner

tcurdt commented Feb 25, 2017

@TomyLobo thanks for the quick reply. I guess we need to check master for that then. At least we should be able to influence this now with the updated commons compress.

@peternewman
Copy link
Contributor

peternewman commented Jun 21, 2017

Any progress on this @tcurdt ? It looks like the md5sum ( https://github.com/tcurdt/jdeb/blame/master/src/main/java/org/vafer/jdeb/DataBuilder.java#L164 ) is using getName, but it's still running fixPath ( https://github.com/tcurdt/jdeb/blame/master/src/main/java/org/vafer/jdeb/DataBuilder.java#L135 ) against it, which effectively breaks the path for what md5sum requires.

I'm using it via https://github.com/nebula-plugins/gradle-ospackage-plugin which seems to currently be on jdeb 1.4, but it doesn't look like the jdeb end is sorted yet, so not much point asking them to update yet.

@tcurdt
Copy link
Owner

tcurdt commented Jun 21, 2017

@peternewman if you are able provide a patch I am happy to apply it and make a well needed release.

@peternewman
Copy link
Contributor

Is there a test case for the conffiles stuff, I couldn't immediately spot one?

I've fixed the md5sum stuff in #259 .

@tcurdt
Copy link
Owner

tcurdt commented Jun 25, 2017

I will double check things when looking into making the release - but looking good so far. Thanks for the contribution. I don't think there is a conffiles testcase yet. Feel free to come up with one :)

@tcurdt tcurdt closed this as completed Jun 25, 2017
@peternewman
Copy link
Contributor

Okay, it's just @TomyLobo seemed to be suggesting conffiles weren't right. Although having looked at the last one we've generated which used jdeb, they seem to be okay to me.

@peternewman
Copy link
Contributor

I can see mention of a conffile here, but don't know what it's generating/where it's tested:

<data src="org/vafer/jdeb/deb/data" type="directory" conffile="true" />

@TomyLobo
Copy link

TomyLobo commented Jun 25, 2017

conffiles needs to have leading slashes.
Since your PR removes leading slashes, I doubt it fixed the issue I mentioned.
Might not even be related to the original issue, now that I think about it...

EDIT:
actually it very much is. See the OP from the line containing "/etc/default" onward

@peternewman
Copy link
Contributor

Hi @TomyLobo ,

The PR I made only impacted on the .md5sums file entries, which shouldn't have a slash (based on ones directly from the Ubuntu repos). conffiles should have the full path though, which it seems to for files generated via https://github.com/nebula-plugins/gradle-ospackage-plugin and jdeb 1.4.

I'm personally keen to get this all sorted, but from the small amount of testing I'd done, it seems like conffiles is currently (well with jdeb 1.4 was) correct, unless you feel otherwise.

@TomyLobo
Copy link

I can confirm that with bash from yakkety:

{ Downloads }  » ar p bash_4.3-15ubuntu1.1_amd64.deb.ar control.tar.gz | tar xz -O ./md5sums
e61108aff394a2d6f1c7cd2efdc097ab  bin/bash
313540054c0cc558a1af59917a925cf6  usr/bin/bashbug
be887f934bcf99a362b95e93f1defcc5  usr/bin/clear_console
53f5fcc072d73bd3a9c3730961492373  usr/share/doc/bash/COMPAT.gz
ab78b78be766692463cb112b88d5a74f  usr/share/doc/bash/INTRO.gz
9df11d534062836a58f82ddad0df6c72  usr/share/doc/bash/NEWS.gz
edea2eee495d96911e81e164ddc891ad  usr/share/doc/bash/POSIX.gz
632b3fbdfd170709a7e4c3724624bf33  usr/share/doc/bash/RBASH
e800272ac95eade956214be4eacba8cf  usr/share/doc/bash/README
6bd7de8b98911613a536e83867e9b490  usr/share/doc/bash/README.Debian.gz
5dac7b9b6332d9845e315cf8fd50ea89  usr/share/doc/bash/README.abs-guide
007dea9b8141f038c602b23f78509e34  usr/share/doc/bash/README.commands.gz
dc59485b41bd9fbdc91f687d4f39ab8f  usr/share/doc/bash/changelog.Debian.gz
9632d707e9eca8b3ba2b1a98c1c3fdce  usr/share/doc/bash/copyright
244319c9dd88c980910aacd76477b8d9  usr/share/doc/bash/inputrc.arrows
4574de6676d74019761f409168aa8e01  usr/share/lintian/overrides/bash
a98d97945a36f4c01508fa82d0235ef1  usr/share/man/man1/bash.1.gz
ab8320c478c9d17caaa4d86e113cf0a2  usr/share/man/man1/bashbug.1.gz
0c912132bdbce02861669392deb3f84c  usr/share/man/man1/clear_console.1.gz
6ad61b838c1370d3bed5d4410644f34a  usr/share/man/man1/rbash.1.gz
b2fb88f251700c29d638d9202e89a693  usr/share/man/man7/bash-builtins.7.gz
0c05a14279f95fdb4618a4ccaa469681  usr/share/menu/bash

However, here's conffiles from the same package:

{ Downloads }  » ar p bash_4.3-15ubuntu1.1_amd64.deb.ar control.tar.gz | tar xz -O ./conffiles
/etc/bash.bashrc
/etc/skel/.bash_logout
/etc/skel/.bashrc
/etc/skel/.profile

@tcurdt
Copy link
Owner

tcurdt commented Jun 26, 2017

@TomyLobo which should be in line with what jdeb does (now).

@TomyLobo
Copy link

both? nice!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants