Re: [io] Regression in FileUtils.copyDirectory() for Windows?

[Gary Gregory]

Thank you Tres, I just approved the PR jobs on GitHub.

Gary

On Mon, Aug 29, 2022, 10:40 Tres Finocchiaro 
wrote:

> https://github.com/apache/commons-io/pull/377
>

Re: [io] Regression in FileUtils.copyDirectory() for Windows?

[Tres Finocchiaro]

https://github.com/apache/commons-io/pull/377

Re: [io] Regression in FileUtils.copyDirectory() for Windows?

[Tres Finocchiaro]

PR submitted. I've left it in draft per our discussions.  Since I am a
first time contributor, it will need intervention before the PR tasks are
executed.

- tres.finocchi...@gmail.com


On Sun, Aug 28, 2022 at 1:20 PM Gary Gregory  wrote:

> We can create a ticket later. If you want to create one now, that's
> fine too.
>
> Gary
>
> On Sun, Aug 28, 2022, 12:21 Tres Finocchiaro 
> wrote:
>
> > >
> > > Please create a PR on GitHub so we can see build results in all the OSs
> > and
> > > Java versions we test there.
> > >
> >
> > Sure thing. The instructions say to reference a JIRA ticket number in the
> > subject line.  Should I create a formal bug report first or is a
> standalone
> > PR sufficient?
> >
> > >
> >
>

Re: [io] Regression in FileUtils.copyDirectory() for Windows?

[Gary Gregory]

We can create a ticket later. If you want to create one now, that's
fine too.

Gary

On Sun, Aug 28, 2022, 12:21 Tres Finocchiaro 
wrote:

> >
> > Please create a PR on GitHub so we can see build results in all the OSs
> and
> > Java versions we test there.
> >
>
> Sure thing. The instructions say to reference a JIRA ticket number in the
> subject line.  Should I create a formal bug report first or is a standalone
> PR sufficient?
>
> >
>

Re: [io] Regression in FileUtils.copyDirectory() for Windows?

[Tres Finocchiaro]

>
> Please create a PR on GitHub so we can see build results in all the OSs and
> Java versions we test there.
>

Sure thing. The instructions say to reference a JIRA ticket number in the
subject line.  Should I create a formal bug report first or is a standalone
PR sufficient?

>

Re: [io] Regression in FileUtils.copyDirectory() for Windows?

[Gary Gregory]

Hi Tres,

Please create a PR on GitHub so we can see build results in all the OSs and
Java versions we test there.

Gary


On Fri, Aug 26, 2022, 01:50 Tres Finocchiaro 
wrote:

> Initial commit(s):
>
> https://github.com/apache/commons-io/compare/master...tresf:commons-io:attributes
>
>- It still needs human testing on many platforms (I plan to perform
>these)
>- Unit tests have been added (need to be human-verified before accepting
>them as passing)
>- I'd like to know why there are two separate function calls for
>preserving the last modified date (I've added this question in a TODO in
>the patch)
>
>
> https://github.com/apache/commons-io/blob/8e072110e9d6ed0f81039929f4a81818c515439c/src/main/java/org/apache/commons/io/FileUtils.java#L2854-L2858
>
> Any/all feedback is welcome.  If the granular approach of "setTimes()"
> hepler (setting creation + modified + access times) is too much of a
> change, the function can be reverted or simplified.
>
>
> - tres.finocchi...@gmail.com
>
>
> On Mon, Aug 22, 2022 at 11:34 AM Tres Finocchiaro <
> tres.finocchi...@gmail.com> wrote:
>
> > > what I'd expect as a new user of Commons IO
> >
> >
> > I think it's safe to say, at least on Windows, the default OS behavior is
> > what 99% of people will want default in a copy utility; copying a file
> to a
> > destination nearly always warrants the destination permissions,
> especially
> > when the ACLs for Windows are so specific and rather tricky to manipulate
> > after the fact.
> >
> > On MacOS, my results are identical, in zsh and bash, a file created in
> > user-space, but copied (NOT moved) to a root-owned space will inherit the
> > "root" permissions upon copy (consistent with old behavior, not new
> > behavior).
> >
> > On Ubuntu, my results are identical as well: In bash, a file created in
> > user-space, but copied (NOT moved) to a root-owned space will inherit the
> > "root" permissions upon copy (consistent with old behavior, not new
> > behavior).
> >
> > In my opinion, the utility is not useful without this behavior toggled
> > back on by default.  Otherwise, NIO with a walktree would accomplish this
> > task instead, so by NOT reverting this change, the project is knowingly
> > abandoning this API (or making a decision to keep an un-intuitive and
> > confusing API workaround.  But I've stated this before, so I will digress
> > in my opinion, it's not really what's being discussed right now.
> >
> > Perhaps a better, more objective argument is that for 16 years, the
> > behavior was the same, only to be changed for (less than) two years.  In
> > the case of my project, this was unnoticed as we often wait before
> > upgrading library versions due to API changes.  (however when Log4J
> > vulnerabilities in past years started reaching our clients, we've been
> > trying to take a more responsible approach)
> >
> > Since this is a "users" list, I would love to hear opinions that are less
> > influenced. :)
> >
> >  > Another concern is that for changes we should have tests that will
> avoid
> >> future regressions and also clearly document why the test exists so that
> >> the tests are not also changed inadvertently,
> >
> >
> > I have been thinking about this as well.  I hope I can receive some
> > guidance on this, however, I would also like to set some scope that I
> would
> > expect:
> >
> >- Javdoc comments updated to be more concise only for the APIs that
> >are modified
> >- I can receive some assistance from experienced devs on writing the
> >unit tests.  I could run platform-specific shell commands or use NIO
> >features, but to test this on Windows would need some form of Windows
> CI
> >host, so hopefully I can get some guidance on GitHub for this.
> >
> > I do understand there's a chance the team will reject this PR for reasons
> > beyond my control, but I also hope that my sentiments about the
> usefulness
> > of this API are weighed, since I believe this API will become less
> > attractive for new developers if not corrected to its old behavior. :)
> >
>

Re: [io] Regression in FileUtils.copyDirectory() for Windows?

[Tres Finocchiaro]

Initial commit(s):
https://github.com/apache/commons-io/compare/master...tresf:commons-io:attributes

   - It still needs human testing on many platforms (I plan to perform
   these)
   - Unit tests have been added (need to be human-verified before accepting
   them as passing)
   - I'd like to know why there are two separate function calls for
   preserving the last modified date (I've added this question in a TODO in
   the patch)

   
https://github.com/apache/commons-io/blob/8e072110e9d6ed0f81039929f4a81818c515439c/src/main/java/org/apache/commons/io/FileUtils.java#L2854-L2858

Any/all feedback is welcome.  If the granular approach of "setTimes()"
hepler (setting creation + modified + access times) is too much of a
change, the function can be reverted or simplified.


- tres.finocchi...@gmail.com


On Mon, Aug 22, 2022 at 11:34 AM Tres Finocchiaro <
tres.finocchi...@gmail.com> wrote:

> > what I'd expect as a new user of Commons IO
>
>
> I think it's safe to say, at least on Windows, the default OS behavior is
> what 99% of people will want default in a copy utility; copying a file to a
> destination nearly always warrants the destination permissions, especially
> when the ACLs for Windows are so specific and rather tricky to manipulate
> after the fact.
>
> On MacOS, my results are identical, in zsh and bash, a file created in
> user-space, but copied (NOT moved) to a root-owned space will inherit the
> "root" permissions upon copy (consistent with old behavior, not new
> behavior).
>
> On Ubuntu, my results are identical as well: In bash, a file created in
> user-space, but copied (NOT moved) to a root-owned space will inherit the
> "root" permissions upon copy (consistent with old behavior, not new
> behavior).
>
> In my opinion, the utility is not useful without this behavior toggled
> back on by default.  Otherwise, NIO with a walktree would accomplish this
> task instead, so by NOT reverting this change, the project is knowingly
> abandoning this API (or making a decision to keep an un-intuitive and
> confusing API workaround.  But I've stated this before, so I will digress
> in my opinion, it's not really what's being discussed right now.
>
> Perhaps a better, more objective argument is that for 16 years, the
> behavior was the same, only to be changed for (less than) two years.  In
> the case of my project, this was unnoticed as we often wait before
> upgrading library versions due to API changes.  (however when Log4J
> vulnerabilities in past years started reaching our clients, we've been
> trying to take a more responsible approach)
>
> Since this is a "users" list, I would love to hear opinions that are less
> influenced. :)
>
>  > Another concern is that for changes we should have tests that will avoid
>> future regressions and also clearly document why the test exists so that
>> the tests are not also changed inadvertently,
>
>
> I have been thinking about this as well.  I hope I can receive some
> guidance on this, however, I would also like to set some scope that I would
> expect:
>
>- Javdoc comments updated to be more concise only for the APIs that
>are modified
>- I can receive some assistance from experienced devs on writing the
>unit tests.  I could run platform-specific shell commands or use NIO
>features, but to test this on Windows would need some form of Windows CI
>host, so hopefully I can get some guidance on GitHub for this.
>
> I do understand there's a chance the team will reject this PR for reasons
> beyond my control, but I also hope that my sentiments about the usefulness
> of this API are weighed, since I believe this API will become less
> attractive for new developers if not corrected to its old behavior. :)
>

Re: [io] Regression in FileUtils.copyDirectory() for Windows?

[Tres Finocchiaro]

>
> > what I'd expect as a new user of Commons IO


I think it's safe to say, at least on Windows, the default OS behavior is
what 99% of people will want default in a copy utility; copying a file to a
destination nearly always warrants the destination permissions, especially
when the ACLs for Windows are so specific and rather tricky to manipulate
after the fact.

On MacOS, my results are identical, in zsh and bash, a file created in
user-space, but copied (NOT moved) to a root-owned space will inherit the
"root" permissions upon copy (consistent with old behavior, not new
behavior).

On Ubuntu, my results are identical as well: In bash, a file created in
user-space, but copied (NOT moved) to a root-owned space will inherit the
"root" permissions upon copy (consistent with old behavior, not new
behavior).

In my opinion, the utility is not useful without this behavior toggled back
on by default.  Otherwise, NIO with a walktree would accomplish this task
instead, so by NOT reverting this change, the project is knowingly
abandoning this API (or making a decision to keep an un-intuitive and
confusing API workaround.  But I've stated this before, so I will digress
in my opinion, it's not really what's being discussed right now.

Perhaps a better, more objective argument is that for 16 years, the
behavior was the same, only to be changed for (less than) two years.  In
the case of my project, this was unnoticed as we often wait before
upgrading library versions due to API changes.  (however when Log4J
vulnerabilities in past years started reaching our clients, we've been
trying to take a more responsible approach)

Since this is a "users" list, I would love to hear opinions that are less
influenced. :)

 > Another concern is that for changes we should have tests that will avoid
> future regressions and also clearly document why the test exists so that
> the tests are not also changed inadvertently,


I have been thinking about this as well.  I hope I can receive some
guidance on this, however, I would also like to set some scope that I would
expect:

   - Javdoc comments updated to be more concise only for the APIs that are
   modified
   - I can receive some assistance from experienced devs on writing the
   unit tests.  I could run platform-specific shell commands or use NIO
   features, but to test this on Windows would need some form of Windows CI
   host, so hopefully I can get some guidance on GitHub for this.

I do understand there's a chance the team will reject this PR for reasons
beyond my control, but I also hope that my sentiments about the usefulness
of this API are weighed, since I believe this API will become less
attractive for new developers if not corrected to its old behavior. :)

Re: [io] Regression in FileUtils.copyDirectory() for Windows?

[Gary Gregory]

On Sun, Aug 21, 2022 at 11:50 AM Tres Finocchiaro <
tres.finocchi...@gmail.com> wrote:

> Unix results have near identical regressions using the stackoverflow
>  snippet, but for
> "/opt/MyApp" (Linux) and "/Applications/MyApp.app" (Mac)
>
> *Ubuntu 22.04:*
>
> 2.8 and lower:
>
>- Files copied from $HOME/Desktop to /opt/MyApp *inherit target
>permissions*.  For example, if you're using copyDirectory() to deploy
>files /opt/, they will generally be owned by the root user.
>
> 2.9 and higher:
>
>- Files copied from $HOME/Desktop to /opt/MyApp *keep source
> permissions*.
>For example, if you're using copyDirectory() to deploy files /opt/, they
>will generally be maintained (in the case of the snippet, owned by the
>*$USER:$USER*).
>
> *MacOS 13.0:*
> 2.8 and lower:
>
>- Files copied from $HOME/Desktop to /opt/MyApp *inherit target
>permissions*.  For example, if you're using copyDirectory() to deploy
>files /Applications/, they will generally be owned by *"root:admin"*.
>
> 2.9 and higher:
>
>- Files copied from $HOME/Desktop to /Applications/MyApp.app *keep
>source permissions* For example, if you're using copyDirectory() to
>deploy files /Applications/, they will generally be maintained (in the
> case
>of the snippet, owned by the "*root:staff*").
>
> Note:
>
> The permissions between Mac and Linux have slightly different behaviors due
> to how Java is handling the creation of the "test.txt" file when using
> "sudo".  Linux creates them with user permissions, Mac does not.
> Regardless, in both cases, the regression seems to behave the same.
> Permission inheritance is lost in both instances.
>

Hi Tres,

Thank you for the analysis :-)


> Should I begin writing a patch to restore the old behavior?
>

That's entirely up to you.

My concern is avoiding a ping-pong of (future) requests for restoring
behavior between released versions. An unexpected change of behavior is not
nice but now that's been released we can't tell if the new behavior is
being relied upon or not.

Another concern is that for changes we should have tests that will avoid
future regressions and also clearly document why the test exists so that
the tests are not also changed inadvertently,

This is why I suggested a new API but that might not be acceptable either.
One way I like to look at this is what I'd expect as a new user of Commons
IO. Maybe we need to extend CopyOption or better Javadoc what we do with
StandardCopyOption?

We should be able to explain clearly in the Javadoc of FileUtils.copyXYZ
and PathUtils.copyXYZ what they do and how they differ and why.

Thank you for digging in!

Gary

Re: [io] Regression in FileUtils.copyDirectory() for Windows?

[Tres Finocchiaro]

Unix results have near identical regressions using the stackoverflow
 snippet, but for
"/opt/MyApp" (Linux) and "/Applications/MyApp.app" (Mac)

*Ubuntu 22.04:*

2.8 and lower:

   - Files copied from $HOME/Desktop to /opt/MyApp *inherit target
   permissions*.  For example, if you're using copyDirectory() to deploy
   files /opt/, they will generally be owned by the root user.

2.9 and higher:

   - Files copied from $HOME/Desktop to /opt/MyApp *keep source permissions*.
   For example, if you're using copyDirectory() to deploy files /opt/, they
   will generally be maintained (in the case of the snippet, owned by the
   *$USER:$USER*).

*MacOS 13.0:*
2.8 and lower:

   - Files copied from $HOME/Desktop to /opt/MyApp *inherit target
   permissions*.  For example, if you're using copyDirectory() to deploy
   files /Applications/, they will generally be owned by *"root:admin"*.

2.9 and higher:

   - Files copied from $HOME/Desktop to /Applications/MyApp.app *keep
   source permissions* For example, if you're using copyDirectory() to
   deploy files /Applications/, they will generally be maintained (in the case
   of the snippet, owned by the "*root:staff*").

Note:

The permissions between Mac and Linux have slightly different behaviors due
to how Java is handling the creation of the "test.txt" file when using
"sudo".  Linux creates them with user permissions, Mac does not.
Regardless, in both cases, the regression seems to behave the same.
Permission inheritance is lost in both instances.

Should I begin writing a patch to restore the old behavior?

Re: [io] Regression in FileUtils.copyDirectory() for Windows?

[Tres Finocchiaro]

I've pinpointed when this regression occurred, it was between 2.8.0 and
2.9.0.

   - (GOOD) 2.4, 2.5, 2.6, 2.7 and 2.8 properly inherit the destination
   permissions.
   - (BAD) 2.8, 2.9, 2.10 and 2.11 do not inherit the destination
   permissions.

Scope of this issue:

   - *2005-2021 (16 years: GOOD behavior)*
   copyDirectory was added in 1.1 and has worked properly up until the
   release of 2.9.

   - *2021-2022 (1.5 years: BAD behavior)*
   Versions since 2.9 have been broken; this bug has only been in
   circulation for as long as 2.9 has been readily available.

First, I'll do some investigation as to what differences this change has on
Unix systems, if any.  I'll be back with my findings.

Re: [io] Regression in FileUtils.copyDirectory() for Windows?

[Gary Gregory]

You're approach is fine. In this case it is better to talk it out here IMO,
before writing code in a PR. I'm not sure it is possible to change the
existing File-based API and have behavior that kinda matches across all
Commons IO versions in a way that will make everyone happy. The good thing
is that you have a way forward that works.

Gary

On Sat, Aug 20, 2022, 16:45 Tres Finocchiaro 
wrote:

> >
> > What about using an empty array as input to PathUtils.copyDirectory(Path,
> > Path, CopyOption...) ?
>
>
> Yes, as long as StandardCopyOptions.*REPLACE_EXISTING* is provided, which
> seems to mimic the former:
>
> PathUtils.copyDirectory(folder.toPath(), dest.toPath(),
> > StandardCopyOption.REPLACE_EXISTING);
>
>
> This new PathUtils API does work as expected.
>
> In my case, I'm already using Path (not File) internally in most places, so
> this could be argued as a slight improvement, however the lack of a
> FileUtils.copyFile(...) equivalent still means we need to -- for now --
> keep around the lingering "false" in places that we copy single files while
> changing their names.
>
> > You can always provide a PR on GitHub to see what a fix would look like.
>
>
> Do you recommend this direct-proposal approach versus submitting a bug
> report and getting buy-in first?
>
> In its current form copying Windows files is (in my opinion) completely
> broken, so I would suspect the the only instances that would complain are:
>
>- Edge-cases with Unix (I would need to test the impact, similar to how
>I used "icacls" on Windows)
>- Windows environments that were accidentally benefiting from this bug,
>which is really people who really wanted COPY_ATTRIBUTES but never
> provided
>this as an option.
>
> The former (to me) is more concerning.
>
> -Tres
>
> - tres.finocchi...@gmail.com
>
>
> On Sat, Aug 20, 2022 at 4:13 PM Gary Gregory 
> wrote:
>
> > Tres,
> >
> > Yeah, I/we got caught here in the desire to provide backward
> compatibility
> > and update the implementation to NIO. You might want to also try the
> > 2.12.0-SNAPSHOT from
> >
> >
> https://repository.apache.org/content/repositories/snapshots/commons-io/commons-io/2.12.0-SNAPSHOT/
> >
> > What about using an empty array as input to PathUtils.copyDirectory(Path,
> > Path, CopyOption...) ?
> >
> > Gary
> >
> > On Sat, Aug 20, 2022 at 3:17 PM Tres Finocchiaro <
> > tres.finocchi...@gmail.com>
> > wrote:
> >
> > > Gary,
> > >
> > > Thanks again for your reply however COPY_ATTRIBUTES is the source of
> the
> > > problem, it's being added and wiping out the permissions.
> > >
> > > I've linked my suspicion as to why in my previous email.
> > >
> > > For now, I will toggle-off the preserveFileDate, which seems to be the
> > only
> > > workaround for this problem.
> > >
> > > Best regards.
> > >
> > >
> > > - tres.finocchi...@gmail.com
> > >
> > > >
> > > >
> > >
> >
>

Re: [io] Regression in FileUtils.copyDirectory() for Windows?

[Tres Finocchiaro]

>
> What about using an empty array as input to PathUtils.copyDirectory(Path,
> Path, CopyOption...) ?


Yes, as long as StandardCopyOptions.*REPLACE_EXISTING* is provided, which
seems to mimic the former:

PathUtils.copyDirectory(folder.toPath(), dest.toPath(),
> StandardCopyOption.REPLACE_EXISTING);


This new PathUtils API does work as expected.

In my case, I'm already using Path (not File) internally in most places, so
this could be argued as a slight improvement, however the lack of a
FileUtils.copyFile(...) equivalent still means we need to -- for now --
keep around the lingering "false" in places that we copy single files while
changing their names.

> You can always provide a PR on GitHub to see what a fix would look like.


Do you recommend this direct-proposal approach versus submitting a bug
report and getting buy-in first?

In its current form copying Windows files is (in my opinion) completely
broken, so I would suspect the the only instances that would complain are:

   - Edge-cases with Unix (I would need to test the impact, similar to how
   I used "icacls" on Windows)
   - Windows environments that were accidentally benefiting from this bug,
   which is really people who really wanted COPY_ATTRIBUTES but never provided
   this as an option.

The former (to me) is more concerning.

-Tres

- tres.finocchi...@gmail.com


On Sat, Aug 20, 2022 at 4:13 PM Gary Gregory  wrote:

> Tres,
>
> Yeah, I/we got caught here in the desire to provide backward compatibility
> and update the implementation to NIO. You might want to also try the
> 2.12.0-SNAPSHOT from
>
> https://repository.apache.org/content/repositories/snapshots/commons-io/commons-io/2.12.0-SNAPSHOT/
>
> What about using an empty array as input to PathUtils.copyDirectory(Path,
> Path, CopyOption...) ?
>
> Gary
>
> On Sat, Aug 20, 2022 at 3:17 PM Tres Finocchiaro <
> tres.finocchi...@gmail.com>
> wrote:
>
> > Gary,
> >
> > Thanks again for your reply however COPY_ATTRIBUTES is the source of the
> > problem, it's being added and wiping out the permissions.
> >
> > I've linked my suspicion as to why in my previous email.
> >
> > For now, I will toggle-off the preserveFileDate, which seems to be the
> only
> > workaround for this problem.
> >
> > Best regards.
> >
> >
> > - tres.finocchi...@gmail.com
> >
> > >
> > >
> >
>

Re: [io] Regression in FileUtils.copyDirectory() for Windows?

[Gary Gregory]

You can always provide a PR on GitHub to see what a fix would look like.
The hard parts are maintaining compatibility with the current version as
opposed to an antique version and updating the tests. Going back to the 2.4
behavior will obviously cause people expecting the 2.11.0 behavior to
complain. Sometimes the only way to address this is (1) a new API or (2)
update the client code to call other APIs with different input.

Gary

On Sat, Aug 20, 2022 at 3:25 PM Tres Finocchiaro 
wrote:

> I've answered my own question on stackoverflow.
>
> I'm curious how the community feels about filing this as a bug.  It's
> pretty big regression for those upgrading from 2.4 with Windows target
> deployments.
>
>
> - tres.finocchi...@gmail.com
>
>
> On Sat, Aug 20, 2022 at 3:16 PM Tres Finocchiaro <
> tres.finocchi...@gmail.com>
> wrote:
>
> > Gary,
> >
> > Thanks again for your reply however COPY_ATTRIBUTES is the source of the
> > problem, it's being added and wiping out the permissions.
> >
> > I've linked my suspicion as to why in my previous email.
> >
> > For now, I will toggle-off the preserveFileDate, which seems to be the
> > only workaround for this problem.
> >
> > Best regards.
> >
> >
> > - tres.finocchi...@gmail.com
> >
> >>
> >>
>

Re: [io] Regression in FileUtils.copyDirectory() for Windows?

[Gary Gregory]

Tres,

Yeah, I/we got caught here in the desire to provide backward compatibility
and update the implementation to NIO. You might want to also try the
2.12.0-SNAPSHOT from
https://repository.apache.org/content/repositories/snapshots/commons-io/commons-io/2.12.0-SNAPSHOT/

What about using an empty array as input to PathUtils.copyDirectory(Path,
Path, CopyOption...) ?

Gary

On Sat, Aug 20, 2022 at 3:17 PM Tres Finocchiaro 
wrote:

> Gary,
>
> Thanks again for your reply however COPY_ATTRIBUTES is the source of the
> problem, it's being added and wiping out the permissions.
>
> I've linked my suspicion as to why in my previous email.
>
> For now, I will toggle-off the preserveFileDate, which seems to be the only
> workaround for this problem.
>
> Best regards.
>
>
> - tres.finocchi...@gmail.com
>
> >
> >
>

Re: [io] Regression in FileUtils.copyDirectory() for Windows?

[Tres Finocchiaro]

I've answered my own question on stackoverflow.

I'm curious how the community feels about filing this as a bug.  It's
pretty big regression for those upgrading from 2.4 with Windows target
deployments.


- tres.finocchi...@gmail.com


On Sat, Aug 20, 2022 at 3:16 PM Tres Finocchiaro 
wrote:

> Gary,
>
> Thanks again for your reply however COPY_ATTRIBUTES is the source of the
> problem, it's being added and wiping out the permissions.
>
> I've linked my suspicion as to why in my previous email.
>
> For now, I will toggle-off the preserveFileDate, which seems to be the
> only workaround for this problem.
>
> Best regards.
>
>
> - tres.finocchi...@gmail.com
>
>>
>>

Re: [io] Regression in FileUtils.copyDirectory() for Windows?

[Tres Finocchiaro]

Gary,

Thanks again for your reply however COPY_ATTRIBUTES is the source of the
problem, it's being added and wiping out the permissions.

I've linked my suspicion as to why in my previous email.

For now, I will toggle-off the preserveFileDate, which seems to be the only
workaround for this problem.

Best regards.


- tres.finocchi...@gmail.com

>
>

Re: [io] Regression in FileUtils.copyDirectory() for Windows?

[Gary Gregory]

Hello Tres,

I think what you want is to call FileUtils.copyDirectory() or
PathUtils.copyDirectory()
with java.nio.file.StandardCopyOption.COPY_ATTRIBUTES.

Gary

On Sat, Aug 20, 2022 at 2:51 PM Tres Finocchiaro 
wrote:

> >
> > Use *org.apache.commons.io.FileUtils.copyFile(File, File, boolean,
> > CopyOption...)* to exercise full control over what you want to do.
> >
>
> Thanks for the reply.
>
> From my understanding, CopyOption supports one or more of three
> options: REPLACE_EXISTING, COPY_ATTRIBUTES, NOFOLLOW_LINKS, none of which
> help with the aforementioned question.  Do you agree?
>
> The behavior of Windows is to preserve the destination permissions, per
> Microsoft:
>
> "By default, an object inherits permissions from its parent object, either
> > at the time of creation or when it is copied or moved to its parent
> folder.
> > The only exception to this rule occurs when you move an object to a
> > different folder on the same volume. In this case, the original
> permissions
> > are retained."
>
>
> So, the default copy behavior is not useful on Windows except
> edge-cases where people are looking to preserve permissions, which is
> non-standard and should not be default.
>
> Under the covers it ends up calling *java.nio.file.Files.copy(Path, Path,
> > CopyOption...)*.
>
>
> I'm happy to try a workaround that uses CopyOption to remedy this, but I'm
> not really sure where to start since the above options don't look like
> they'll help.
>

Re: [io] Regression in FileUtils.copyDirectory() for Windows?

[Tres Finocchiaro]

>
> Under the covers it ends up calling *java.nio.file.Files.copy(Path, Path,
> CopyOption...)*.


Sure, but from what I'm reading, it's adding CopyOptions that no one asked
for and is not in alignment with how NIO works, so this statement is a bit
misleading.

I think this is the problematic line:
https://github.com/apache/commons-io/blob/f22a4227401855ecbfdf8184bbe37275c3aeb5c3/src/main/java/org/apache/commons/io/FileUtils.java#L701

>From what I'm reading, toggling off "preserveFileDate" will
inadvertently and unobviously fix this, but at this point.  This fix seems
like undesired, undocumented and prone-to-break-later behavior, at least
for Windows environments.

I'm curious what reasoning was used to associate a file's modified date
with the rest of its attributes.  Perhaps this was a backward-compat
shortcut to maintain the modified date using the new NIO API?  Regardless,
this introduces permissions changes which can and will break standard file
copy operations for environments which expect inherited permissions, e.g.
Windows.

Ideally, the file modified date could stay without clobbering the default
NIO behavior.

For now a viable workaround is:

FileUtils.copyDirectory(folder, dest, false /* <--- change to FALSE */);


... hjowever this does have the disadvantage of losing timestamp
information, which is still a regression, albeit smaller.

Re: [io] Regression in FileUtils.copyDirectory() for Windows?

[Tres Finocchiaro]

>
> Use *org.apache.commons.io.FileUtils.copyFile(File, File, boolean,
> CopyOption...)* to exercise full control over what you want to do.
>

Thanks for the reply.

>From my understanding, CopyOption supports one or more of three
options: REPLACE_EXISTING, COPY_ATTRIBUTES, NOFOLLOW_LINKS, none of which
help with the aforementioned question.  Do you agree?

The behavior of Windows is to preserve the destination permissions, per
Microsoft:

"By default, an object inherits permissions from its parent object, either
> at the time of creation or when it is copied or moved to its parent folder.
> The only exception to this rule occurs when you move an object to a
> different folder on the same volume. In this case, the original permissions
> are retained."


So, the default copy behavior is not useful on Windows except
edge-cases where people are looking to preserve permissions, which is
non-standard and should not be default.

Under the covers it ends up calling *java.nio.file.Files.copy(Path, Path,
> CopyOption...)*.


I'm happy to try a workaround that uses CopyOption to remedy this, but I'm
not really sure where to start since the above options don't look like
they'll help.

Re: [io] Regression in FileUtils.copyDirectory() for Windows?

[Gary Gregory]

Under the covers it ends up calling *java.nio.file.Files.copy(Path, Path,
CopyOption...)*.

On Sat, Aug 20, 2022 at 2:30 PM Gary Gregory  wrote:

> Hello Tres,
>
> Use *org.apache.commons.io.FileUtils.copyFile(File, File, boolean,
> CopyOption...)* to exercise full control over what you want to do.
>
> Gary
>
>
> On Sat, Aug 20, 2022 at 12:30 PM Tres Finocchiaro <
> tres.finocchi...@gmail.com> wrote:
>
>> Hi,
>>
>> I'm updating some dependent libraries and hit a snag with commons-io.  I
>> was hoping someone here could help.
>>
>> I've noticed a major change in behavior with FileUtils.copyDirectory()
>> between commons-io@2.4 and commons-io@2.11 that affects the permission of
>> files being copied.
>>
>> I've posted the technical details here, with steps to reproduce:
>> https://stackoverflow.com/questions/73428286
>>
>> In short:
>>
>>1. The default 2.4 behavior was to inherit permissions of the target
>>2. The default 2.11 behavior seems to not inherit permissions of the
>>target
>>
>> Any insight is greatly appreciated.
>>
>> - tres.finocchi...@gmail.com
>>
>

Re: [io] Regression in FileUtils.copyDirectory() for Windows?

[Gary Gregory]

Hello Tres,

Use *org.apache.commons.io.FileUtils.copyFile(File, File, boolean,
CopyOption...)* to exercise full control over what you want to do.

Gary


On Sat, Aug 20, 2022 at 12:30 PM Tres Finocchiaro <
tres.finocchi...@gmail.com> wrote:

> Hi,
>
> I'm updating some dependent libraries and hit a snag with commons-io.  I
> was hoping someone here could help.
>
> I've noticed a major change in behavior with FileUtils.copyDirectory()
> between commons-io@2.4 and commons-io@2.11 that affects the permission of
> files being copied.
>
> I've posted the technical details here, with steps to reproduce:
> https://stackoverflow.com/questions/73428286
>
> In short:
>
>1. The default 2.4 behavior was to inherit permissions of the target
>2. The default 2.11 behavior seems to not inherit permissions of the
>target
>
> Any insight is greatly appreciated.
>
> - tres.finocchi...@gmail.com
>