[JENKINS-64621] Fix zip regression

[Wadeck]

See JENKINS-64621.

The zip created from the workspace (or other location related to DirectoryBrowserSupport) contains the parent folder when there is no glob pattern. This behavior was changed during the fix but was not required.

⚠️ At the time of writing this PR, this regression does not seem to have sufficiently important impact to justify an expedite merge/LTS, but please change my mind :)

History of that feature being broken:

• JENKINS-19947, October 2013
• JENKINS-61473, March 2020

⚠️ There are other issues related to this security fix that are not addressed by this PR:

• JENKINS-64632, file descriptor leak
• JENKINS-64655, regression on external storage plugin

Proposed changelog entries

• Include root folder in downloaded zip files. Regression in 2.263.2 (SECURITY-1452)
• Also resolve the previously (before security release) reported issue JENKINS-61473 around the same area.
• (internal) Restricted methods FilePath#zip(OutputStream, DirScanner, String, boolean) and VirtualFile#zip(OutputStream, String, String, boolean, boolean) have a new parameter String prefix.

Proposed upgrade guidelines

N/A

Submitter checklist

• (If applicable) Jira issue is well described
• Changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developer, depending on the change). Examples
  • Fill-in the Proposed changelog entries section only if there are breaking changes or other changes which may require extra steps from users during the upgrade
• Appropriate autotests or explanation to why this change has no tests
• For dependency updates: links to external changelogs and, if possible, full diffs

Desired reviewers

@jeffret-b @daniel-beck @timja

Maintainer checklist

Before the changes are marked as ready-for-merge:

• There are at least 2 approvals for the pull request and no outstanding requests for change
• Conversations in the pull request are over OR it is explicit that a reviewer does not block the change
• Changelog entries in the PR title and/or Proposed changelog entries are correct
• Proper changelog labels are set so that the changelog can be generated automatically
• If the change needs additional upgrade steps from users, upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the PR title. (example)
• If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).

[MarkEWaite]

I built a local copy of jenkins.war with this change and verified that it successfully completes the test that consistently fails with Jenkins 2.275. Thanks for the quick response @Wadeck and @jeffret-b .

[oleg-nenashev]

I confirm it works. Thanks @Wadeck!

[MarkEWaite]

This PR is now ready for merge. We will merge it after approximately 24 hours if there is no negative feedback.

We may choose to merge this one before the full 24 hours has elapsed so that it can be included in the Jenkins 2.276 weekly release Jan 19, 2021.

@jeffret-b is it OK to merge this without the additional test coverage that you had implemented? I assume the additional test coverage can be merged later, after adapting it to the API changes in this pull request.

[noamran]

At the time of writing this PR, this regression does not seem to have sufficiently important impact to justify an expedite merge/LTS, but please change my mind :)

Thanks for adding this fix! Just chiming in and adding that this regression does have a significant impact on us and this is preventing us from moving to 2.263.2. We would appreciate an expedited merge/LTS if possible.

[MarkEWaite]

@noamran have you verified that the change resolves your issue?

Have you done any exploratory testing to see if there are unexpected results from the change?

[daniel-beck]

History of that feature being broken:

• JENKINS-61473, March 2020

FTR still unresolved in 2.274, but only affected agent workspaces.

[daniel-beck]

That seems premature TBH, but 🤷

[timja]

That seems premature TBH, but 🤷

build passed previously with the same code (since it was reverted), and weekly will go out automatically in the next few hours

[daniel-beck]

Appears to also resolve JENKINS-61473. Could someone confirm and update the changelog suggestion?

[jeffret-b]

This looks good, though I haven't tested it. I have a more complete set of automated tests I'd like to run with it, but it will take a bit to get them properly targeted and running again. I don't think it's getting into the more troublesome areas, though, so it's probably fine.