Skip to content

Commit c5f5aed

Browse files
committed
fix: add null-guard for $this->membership in downgrade cart type paths
When custom/out-of-tree code sets cart_type to 'downgrade' without calling build_from_membership(), $this->membership can be null, causing a fatal NPE in get_billing_start_date() and get_billing_next_charge_date(). Add a Yoda-style null check before dereferencing $membership in both downgrade blocks: - get_billing_start_date(): return null on null membership - get_billing_next_charge_date(): return $smallest_next_charge on null membership Add two tests using an anonymous subclass that overrides build_cart() to simulate the out-of-tree NPE scenario. Resolves #897
1 parent ddf4b69 commit c5f5aed

2 files changed

Lines changed: 73 additions & 0 deletions

File tree

inc/checkout/class-cart.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2706,6 +2706,10 @@ public function get_billing_start_date() {
27062706
if ($this->get_cart_type() === 'downgrade') {
27072707
$membership = $this->membership;
27082708

2709+
if (null === $membership) {
2710+
return null;
2711+
}
2712+
27092713
if ($membership->is_active() || $membership->get_status() === Membership_Status::TRIALING) {
27102714
return strtotime($membership->get_date_expiration());
27112715
}
@@ -2747,6 +2751,10 @@ public function get_billing_next_charge_date() {
27472751
if ($this->get_cart_type() === 'downgrade') {
27482752
$membership = $this->membership;
27492753

2754+
if (null === $membership) {
2755+
return $smallest_next_charge;
2756+
}
2757+
27502758
if ($membership->is_active() || $membership->get_status() === Membership_Status::TRIALING) {
27512759
$next_charge = strtotime($membership->get_date_expiration());
27522760

tests/WP_Ultimo/Checkout/Cart_Test.php

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2859,6 +2859,71 @@ public function test_reactivation_cart_rebuilds_products_from_membership() {
28592859
$injected_plan->delete();
28602860
}
28612861

2862+
/**
2863+
* Test that get_billing_start_date() returns null when cart_type is 'downgrade'
2864+
* but $this->membership is null (out-of-tree cart_type override without build_from_membership()).
2865+
*
2866+
* Simulates the scenario where a subclass or addon sets cart_type to 'downgrade'
2867+
* via a hook/override without having called build_from_membership(), leaving
2868+
* $this->membership as null.
2869+
*
2870+
* @covers \WP_Ultimo\Checkout\Cart::get_billing_start_date
2871+
*/
2872+
public function test_billing_start_date_null_guard_on_downgrade_without_membership() {
2873+
$plan = $this->create_plan([
2874+
'amount' => 50.00,
2875+
'duration' => 1,
2876+
'duration_unit' => 'month',
2877+
]);
2878+
2879+
// Anonymous subclass overrides build_cart() to force cart_type='downgrade'
2880+
// without setting $this->membership, reproducing the out-of-tree NPE scenario.
2881+
$cart = new class(['products' => [$plan->get_id()]]) extends Cart {
2882+
protected function build_cart() {
2883+
$this->cart_type = 'downgrade';
2884+
// $this->membership intentionally left null to trigger the guard.
2885+
}
2886+
};
2887+
2888+
// membership is null because build_from_membership() was never called.
2889+
$this->assertNull($cart->get_billing_start_date(), 'get_billing_start_date() must not NPE when cart_type is downgrade but membership is null');
2890+
}
2891+
2892+
/**
2893+
* Test that get_billing_next_charge_date() returns a valid integer when cart_type is
2894+
* 'downgrade' but $this->membership is null (out-of-tree cart_type override without
2895+
* build_from_membership()).
2896+
*
2897+
* Simulates the scenario where a subclass or addon sets cart_type to 'downgrade'
2898+
* via a hook/override without having called build_from_membership(), leaving
2899+
* $this->membership as null.
2900+
*
2901+
* @covers \WP_Ultimo\Checkout\Cart::get_billing_next_charge_date
2902+
*/
2903+
public function test_billing_next_charge_date_null_guard_on_downgrade_without_membership() {
2904+
$plan = $this->create_plan([
2905+
'amount' => 50.00,
2906+
'duration' => 1,
2907+
'duration_unit' => 'month',
2908+
]);
2909+
2910+
// Anonymous subclass overrides build_cart() to force cart_type='downgrade'
2911+
// without setting $this->membership, reproducing the out-of-tree NPE scenario.
2912+
$cart = new class(['products' => [$plan->get_id()]]) extends Cart {
2913+
protected function build_cart() {
2914+
$this->cart_type = 'downgrade';
2915+
// $this->membership intentionally left null to trigger the guard.
2916+
}
2917+
};
2918+
2919+
// membership is null; the guard should fall through and return $smallest_next_charge
2920+
// (a large future timestamp indicating "next charge not yet determined").
2921+
$result = $cart->get_billing_next_charge_date();
2922+
2923+
$this->assertIsInt($result, 'get_billing_next_charge_date() must return an int when cart_type is downgrade but membership is null');
2924+
$this->assertGreaterThan(time(), $result, 'get_billing_next_charge_date() must return a future timestamp when membership is null');
2925+
}
2926+
28622927
public static function tear_down_after_class() {
28632928
global $wpdb;
28642929
self::$customer->delete();

0 commit comments

Comments
 (0)