Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#41889 closed defect (bug) (invalid)

Missing Break Statement In class-pclzip php file

Reported by: adnanlimdi's profile adnan.limdi Owned by:
Milestone: Priority: low
Severity: normal Version:
Component: Filesystem API Keywords: has-patch 2nd-opinion
Focuses: Cc:

Description

Missing Break statement in switch case conditions it is PHP warning.

Attachments (1)

41889.patch (503 bytes) - added by adnan.limdi 7 years ago.
Missing break Statement in switch case.

Download all attachments as: .zip

Change History (9)

@adnan.limdi
7 years ago

Missing break Statement in switch case.

#1 @adnan.limdi
7 years ago

  • Keywords has-patch added

This ticket was mentioned in Slack in #core by adnanlimdi. View the logs.


7 years ago

#3 follow-up: @jrf
7 years ago

@adnan.limdi Could you provide the error message and a backtrace of it please ?

#4 in reply to: ↑ 3 @adnan.limdi
7 years ago

Replying to jrf:

@adnan.limdi Could you provide the error message and a backtrace of it please ?

@jrf i found warning when dubug using php code stander this file in switch case statement. and i got this message : Break Statement is missing.

Last edited 7 years ago by adnan.limdi (previous) (diff)

#5 @johnbillion
7 years ago

  • Component changed from General to Filesystem API
  • Focuses docs removed
  • Keywords 2nd-opinion added
  • Priority changed from normal to low

A missing break statement does not automatically indicate an error in the code. An expression can match multiple case conditions and intentionally fall through (this happens in the large switch statement in the map_meta_cap() function).

In this case, it does look like the break statement is missing, but I don't believe there is a reason to "fix" this unless a bug can be identified.

#6 @dd32
7 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed

This looks to be intentional to me.

PCLZIP_OPT_BY_EREG is deprecated, so it overwrites PCLZIP_OPT_BY_EREG with PCLZIP_OPT_BY_PREG in the args array, and then performs the validation for the parameters for ..PREG which is what now happens when you use ..EREG

Code in question with more context:
https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/class-pclzip.php?marks=1566-1579#L1558

It's also worth noting, that PclZip is not a WordPress library, it's home is over at http://www.phpconcept.net/pclzip and bugs in it can be reported directly to the author. Over the years we've made some updates to the library for PHP compatibility, but that's it.

Unless this is causing a PHP Warning or related message or an actual bug, I'm marking this as invalid. A warning from a code sniffer or similar tool isn't enough of a reason to change it IMHO.

#7 @adnan.limdi
7 years ago

@dd32 Thanks for providing information for that. I take care next time about PHP warning.

#8 @jrf
7 years ago

@johnbillion @dd32 Should an // Intentionally falling through to next case. comment be added maybe to make it indisputable that this in the intended behaviour ? /cc @DrewAPicture

Note: See TracTickets for help on using tickets.