WordPress.org

Make WordPress Core

Opened 15 months ago

Last modified 8 weeks ago

#46418 reviewing enhancement

Use wp_die() instead of die() function in wordpress

Reported by: immeet94 Owned by: SergeyBiryukov
Milestone: 5.5 Priority: normal
Severity: normal Version:
Component: Administration Keywords: has-patch commit
Focuses: Cc:

Description

When use the wp_json_encode() function that time you should use wp_die function instead of die() function.
Because wp_die() function do Kill WordPress execution and display HTML message with error message.

Attachments (6)

#46418.diff (461 bytes) - added by immeet94 15 months ago.
Please see this file for patch
46418.2.diff (1.3 KB) - added by garrett-eclipse 12 months ago.
Updated all instances of die to wp_die in class-wp-list-table.php
46418.3.diff (1.3 KB) - added by garrett-eclipse 7 months ago.
Refresh for 5.4, same code just reduces hunk offsets where were exceeding 25 lines.
48676.4.diff (1.2 KB) - added by garrett-eclipse 7 months ago.
Refresh to account for commit#46739
46418.5.diff (21.6 KB) - added by immeet94 8 weeks ago.
46418.6.diff (23.0 KB) - added by immeet94 8 weeks ago.

Download all attachments as: .zip

Change History (19)

@immeet94
15 months ago

Please see this file for patch

#1 @immeet94
15 months ago

  • Severity changed from normal to major

#2 @immeet94
15 months ago

  • Severity changed from major to normal
  • Version trunk deleted

#3 @desrosj
12 months ago

  • Component changed from General to Administration
  • Keywords needs-refresh added
  • Milestone changed from Awaiting Review to Future Release

Hi @immeet94,

Welcome to Trac!

Thanks for this ticket. It seems that there are several more occurrences of die() in WP_List_Table. Are you able to look into fixing those as well?

@garrett-eclipse
12 months ago

Updated all instances of die to wp_die in class-wp-list-table.php

#4 @garrett-eclipse
12 months ago

  • Keywords needs-testing added; needs-refresh removed

In 46418.2.diff I've replaced all instances of die with wp_die in class-wp-list-table.php

#5 @sncoker
7 months ago

At the #WCUS Contributor Day. The patch applies cleanly to trunk.

#6 @SergeyBiryukov
7 months ago

  • Milestone changed from Future Release to 5.4
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

@garrett-eclipse
7 months ago

Refresh for 5.4, same code just reduces hunk offsets where were exceeding 25 lines.

#7 @garrett-eclipse
7 months ago

  • Keywords commit added; needs-testing removed

Thanks for testing @sncoker I uploaded 46418.3.diff just because the offset was starting to get larger.

Marking for committer review, @SergeyBiryukov I see you were just looking at this as well, will leave in your hands.

Sidenote - In the affected lines I noticed the use of sub-class and over-ridden which are incorrect and should be subclass and overridden. To address this I've setup #48676 as those strings are seen in several other files as well so best to address seperately.

@garrett-eclipse
7 months ago

Refresh to account for commit#46739

#8 @garrett-eclipse
7 months ago

With r46739 updating the version being committed (thanks @SergeyBiryukov) I've updated the patch in 48676.4.diff.

While looking at this I wonder what other instances of die in core should become wp_die;
wp-activate.php line#18
wp-admin/admin-footer.php line#11
wp-admin/async-upload.php line#34
wp-admin/comment.php lines#95,100,106
wp-admin/edit-form-advanced.php line#11
wp-admin/edit-form-blocks.php line#13
wp-admin/edit-form-comment.php line#11
wp-admin/edit-link-form.php line#11
wp-admin/edit-tag-form.php line#11
wp-admin/export.php line#122
wp-admin/includes/class-pclzip.php line#221
wp-admin/includes/class-wp-site-health.php line#196
wp-admin/includes/image-edit.php line#28
wp-admin/includes/network.php lines#120,133,148
wp-admin/install.php lines#217,264,269,282
wp-admin/link-parse-opml.php line#10
wp-admin/upgrade.php line#35
wp-admin/user-edit.php lines#109,117
wp-admin/user-new.php lines#44,50,150,174,217
wp-cron.php lines#32,81,152
wp-includes/canonical.php lines#349,394,611
wp-includes/class-simplepie.php line#665
wp-includes/class-wp-ajax-response.php line#160
wp-includes/class-wp-widget.php line#113
wp-includes/comment.php line#716,930
wp-includes/embed.php line#728
wp-includes/functions.php lines#3419,3459,3499,3541,3573,3617,3639,3641,4513
wp-includes/IXR/class-IXR-server.php line#47
wp-includes/load.php lines#35,219,614,1449
wp-includes/ms-deprecated.php lines#116,615
wp-includes/ms-files.php lines#15,24,30
wp-includes/pluggable.php lines#1130,1184
wp-includes/rest-api.php line#308
wp-includes/theme-compat/comments.php line#20
wp-includes/wp-db.php line#1678
wp-settings.php line#530
wp-signup.php lines#14,34,39,412
wp-trackback.php line#35

#9 @davidbaumwald
4 months ago

@SergeyBiryukov The latest patch still good. Is this still a commit candidate for 5.4 with Beta 1 in a few days?

#10 @SergeyBiryukov
4 months ago

  • Milestone changed from 5.4 to 5.5

#11 @immeet94
8 weeks ago

Hello

as per the @garrett-eclipse suggestion try to replace the wp_die() in all files so please review the patch @SergeyBiryukov and @davidbaumwald

Thanks.

@immeet94
8 weeks ago

#12 @garrett-eclipse
8 weeks ago

Thanks @immeet94 taking a look one thing to be cautious about is when wp_die the function becomes available which is in wp-includes/functions.php. In index.php the wp-includes/load.php is required before the functions one so the changes there may fail so I would ensure especially the changes in wp-includes/load.php are tested thoroughly.

#13 @immeet94
8 weeks ago

Hello @garrett-eclipse

i miss the file wp-includes/functions.php for the wp_die so updated with wp_die

please check the patch

thanks

@immeet94
8 weeks ago

Note: See TracTickets for help on using tickets.