Making WordPress.org

Opened 7 years ago

Last modified 5 months ago

#3263 reviewing defect (bug)

Zips for deleted plugin tags not deleted

Reported by: ipstenu's profile Ipstenu Owned by:
Milestone: Plugin Directory v3 - Future Priority: normal
Component: Plugin Directory Keywords: needs-patch
Cc:

Description

When you delete a plugin tag, the resultant zip that had previously been generated is not removed.

This causes older versions to sit out there and be downloadable, even when they shouldn’t be.

Example.

https://downloads.wordpress.org/plugin/impostercide.1.6.zip

The tag folder was removed on Nov 10.

(I know this is too soon to expect it deleted, but it’s an example)

Change History (11)

#1 @tellyworth
7 years ago

  • Milestone set to Plugin Directory v3 - Future

#2 @obenland
6 years ago

  • Owner set to dd32
  • Status changed from new to reviewing

Is this something you'd consider low/medium effort?

#3 @dd32
6 years ago

Not entirely sure, but I wouldn't call this low effort, it's also likely to have some issues around race conditions of people deleting tags and re-creating them.

I don't personally believe we need to delete these ZIPs either though, as keeping them around causes no issues.

#4 @dd32
6 years ago

After thinking more about this, I don't think we should automatically delete generated packages, even if possible.

Having the archives hanging around isn't an issueb with disk usage, and they were only deleted in the past as all archives were regenerated every commit.

The only scenario I can think of where deleting packages would be needed, are where it has security or IP concerns. For those cases we could have either a bin script or a purge option in the back end.

#5 follow-up: @Otto42
6 years ago

@dd32 Is there any scenario where we would want to have the downloader process check for "closed" and disallow the download of the ZIP file? We don't necessarily need to delete the ZIPs, but we may want to prevent closed plugins from being able to be downloaded.

My concern is that this would add overhead to the download process by checking the database, but that may or may not be a valid concern.

#6 in reply to: ↑ 5 @dd32
6 years ago

Replying to Otto42:

@dd32 Is there any scenario where we would want to have the downloader process check for "closed" and disallow the download of the ZIP file?

I don't think so. The content of the zips is all public svn data anyway.

My concern is that this would add overhead to the download process by checking the database

Yeah, we don't want that overhead in there. It'd be likely that at some future time the downloads would be served by a proper CDN where such checks would be even more of a significant overhead.

#7 follow-up: @Ipstenu
6 years ago

The content of the zips is all public svn data anyway.

Are the zips deleted when SVN is zero'd out?

Thinking of reasons why we close plugins, sometimes SVN gets wiped out (security, copyright, etc) and the zips should be nuked too.

#8 in reply to: ↑ 7 @dd32
6 years ago

Replying to Ipstenu:

The content of the zips is all public svn data anyway.

Are the zips deleted when SVN is zero'd out?

That's what this ticket is about.
If a tag is deleted, rather than just being zero'd out, then the zip remains untouched.

I remain in the camp that we shouldn't expend effort on removing the zips in those cases. Especially not when the ZIPs are not linked to from anywhere.

Thinking of reasons why we close plugins, sometimes SVN gets wiped out (security, copyright, etc) and the zips should be nuked too.

The data is still available in SVN though, just not visible via the HTTP browser.
In most cases, having a ZIP still be available is not a huge concern in my mind. Except as mentioned above, cases of Copyright and/or IP issues.

#9 follow-up: @Ipstenu
6 years ago

If a tag is deleted, rather than just being zero'd out, then the zip remains untouched.

Well that's no good. Most people are going to delete a tag to resolve a copyright issue (or in the case of one very silly person, uploading credit card information...)

In most cases, having a ZIP still be available is not a huge concern in my mind. Except as mentioned above, cases of Copyright and/or IP issues.

Right and since there's absolutely no way to easily flag them, that's why I think we should auto-delete them. Unless you want to set up a process by which I get to contact you/otto every time I close a plugin for copyright/security to make sure we don't push bad/illegal/violation code.

And that's assuming I did that in the first place. Sometimes people catch on and fix things themselves.

I agree that the race condition issue a big one. Would it be possible to run a weekly/bi-weekly/monthly script for plugins that compares tags to zips, and if there's no tag, nuke the zip? Or would that be worse?

#10 in reply to: ↑ 9 @dd32
6 years ago

  • Owner dd32 deleted

Replying to Ipstenu:

Would it be possible to run a weekly/bi-weekly/monthly script for plugins that compares tags to zips, and if there's no tag, nuke the zip? Or would that be worse?

Much worse.

If anything happens here, I guess it'd have to be something like this to avoid the most common race conditions:

  • When a commit deleting a tag occurs
    • Queue a cron task to check that the tag really doesn't exist an hour later
    • In Cron: If no tag. Remove zip

The SVN Watcher cannot handle this case at present, and the zip management stuff includes no ability to delete due to never-deleting being a design goal of the new zip storage system.

#11 @dd32
5 months ago

In 13709:

Plugin Directory: SVN: Watch for Tag deletions, such that we can properly handle cases when a tag is deleted.

See #3263, #5900.

Note: See TracTickets for help on using tickets.