Make WordPress Core

Opened 10 years ago

Last modified 5 years ago

#27758 new defect (bug)

WP_Error data is false in _unzip_file_ziparchive

Reported by: ruudjoyo's profile ruud@… Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.7
Component: Upgrade/Install Keywords: needs-patch
Focuses: Cc:

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
10 years ago

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

#2 @nacin
10 years ago

That line of code was designed to avoid disclosing the absolute file path, as this error data is actually aggregated by 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@…
10 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 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

#4 @ruud@…
10 years ago

I assumed with 'actually aggregated by' that you meant that the error codes where somehow submitted/returned to 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
10 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(
     __( 'Could not create directory.' ),
     ( $_dir == $to ) ? str_replace( dirname( WP_CONTENT_DIR ), '', $_dir ) : substr( $_dir, strlen( $to ) )

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

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

#6 @chriscct7
9 years ago

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