SMF icon indicating copy to clipboard operation
SMF copied to clipboard

Improve tgz file handling

Open live627 opened this issue 2 years ago • 12 comments

  • Fixes #7227

live627 avatar Jan 09 '22 22:01 live627

Is this compatible with #7228 or an alternative fix?

jdarwood007 avatar Jan 09 '22 22:01 jdarwood007

This is alternative fix

live627 avatar Jan 10 '22 01:01 live627

@live627 - two things:

  • It doesn't fix the initial problem, I am still seeing the same/similar issues when the OSX files are present, e.g., WSOD in the Package Manager: image

I am also seeing issues trying to analyze a package for the cust site - slightly different, though, it cannot find package-info.xml at all.

  • I am getting thousands of errors in the error log: 2: Undefined array key "filename" in Subs-Package, various lines, including 210, 206, 186, 178

I used the same test file we were using on the cust site with the OSX files. (Since I don't have a mac I need to use @jdarwood007 's example...)

sbulen avatar Jan 10 '22 01:01 sbulen

I saw you pushed some commits while I was writing up my earlier test results.

I just retested, and all the issues reported above appear to have been addressed.

I must admit, though, that rewrites of stable code always make me nervous... One of my most repeated complaints with PRs here...

sbulen avatar Jan 10 '22 01:01 sbulen

(When the destination is null, that is a request for a list of files in the archive.)

sbulen avatar Jan 10 '22 01:01 sbulen

If both fix the same thing, then I guess its a mater of choice.

This one feels like it may be more complete in trying to handle the tar file according to the RFC.

However we are late in 2.1 stages. I see it also added hints. While I like type hints, we have to be very careful of their usage in 2.1, because of so many places we could possibly break it. This PR also may need additional testing for open_basedir restriction testing, which the $destination !== null checks fixed by preventing us from doing file checks on directories that would start from a path most likely outside of the allowed dirs.

In all honesty, I think #7228 should be merged for 2.1. This one should go into the future because it is more complete into the RFC spec (as I see it handling some additional types other than files). @Sesquipedalian may want to make that decision though.

jdarwood007 avatar Jan 10 '22 01:01 jdarwood007

Testing this one is pretty much the same as https://github.com/SimpleMachines/SMF2.1/pull/7180#issuecomment-994229770

live627 avatar Jan 18 '22 23:01 live627

@live627 - I'm not sure this is a hard requirement for 2.1.0?

sbulen avatar Jan 21 '22 02:01 sbulen

Not important enough to hold up the release. The excising code works fine with the ubiquitous ustar format, just not the "newer" pax format.

I'll let @Sesquipedalian assign a new milestone as he sees fit.

live627 avatar Jan 21 '22 08:01 live627

I'd rather merge the full fix. However, there's really no hurry for dealing with this issue. I've been making SMF compatible tar.gz files on my Mac for years; it's just a matter of setting the right command line switches. Anyway, we can take the time to test this PR as thoroughly as we like after 2.1.0 is out.

Sesquipedalian avatar Jan 24 '22 00:01 Sesquipedalian

FYI, this didn't get merged into 2.1.0. Can we do the #7228 for 2.1.x?

jdarwood007 avatar Feb 09 '22 23:02 jdarwood007

The speedup mentioned in the latest commit is 0.2 seconds with a package directory filled with 89 objects; package directories, php files, package files.

live627 avatar Jul 09 '23 10:07 live627