WordPress.org

Make WordPress Core

Opened 6 years ago

Last modified 6 months ago

#27758 new defect (bug)

WP_Error data is false in _unzip_file_ziparchive

Reported by: ruud@… Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.7
Component: Upgrade/Install Keywords: needs-patch
Focuses: Cc:
PR Number:

Description (last modified by SergeyBiryukov)

This is a follow-up to #22704, and the short version because after my long version crashed on 'continue to preview' :(

return new WP_Error( 'mkdir_failed_ziparchive', __( 'Could not create directory.' ), substr( $_dir, strlen( $to ) ) );

substr( $_dir, strlen( $to ) ) results in false for the 'upgrade' folder and the zipfile folder itself. For the other folders the 'data' is simply not very informative and could be even confusing.

Before [25780], it used to be just $_dir; isn't that a much more informative feedback?

Change History (6)

#1 @SergeyBiryukov
6 years ago

  • Description modified (diff)
  • Version changed from 3.8.1 to 3.7

#2 @nacin
6 years ago

That line of code was designed to avoid disclosing the absolute file path, as this error data is actually aggregated by WordPress.org. So whatever we do here (if it is indeed broken) we need to make sure the error data isn't an absolute path.

#3 @ruud@…
6 years ago

Hi Sergey and Nacin,
Thanks for the quick response, and apologies for the rather blunt bug report.
I can also fully understand that this issue keeps dormant for now, especially with the upcoming release ahead.

That said, I had a quick thought on this: How about the error data when its actually send/submitted/post to wordpress.org is then filtered for absolute paths and perhaps other stuff? If that's possible than the local site owner still gets his full feedback, but no delicate info is aggregated by wordpress.org

#4 @ruud@…
6 years ago

I assumed with 'actually aggregated by WordPress.org' that you meant that the error codes where somehow submitted/returned to wordpress.org for analysis and such. However I may have misunderstood that because the code I've looked through did not give me more clues in that direction.
So my attempt in seeing if the 'filter' I proposed is halted for now.

If there is such place where this could happen, please tell me so I can have a look.

#5 @dd32
5 years ago

One option here is to simply strip WP_CONTENT_DIR out of the $_dir, another is to just use the basename() but that'll be possibly just as confusing.

For example, this would return 'wp-content/upgrade' as the error data if it failed to create that directory:

return new WP_Error(
     'mkdir_failed_ziparchive',
     __( 'Could not create directory.' ),
     ( $_dir == $to ) ? str_replace( dirname( WP_CONTENT_DIR ), '', $_dir ) : substr( $_dir, strlen( $to ) )
);

I assumed with 'actually aggregated by WordPress.org' that you meant that the error codes where somehow submitted/returned to wordpress.org for analysis and such.

That is correct, it's handled far above the underlying upgrade functions for things like auto updates.

#6 @chriscct7
4 years ago

  • Keywords needs-patch added
Note: See TracTickets for help on using tickets.