Further to #1878674: Incorrect usage of order_number - this issue still appears to be present in the unified SagePay project, but it's more endemic than just the 3D secure module - there's several examples of redirecting to checkout/$order->order_number, and at least one example where a commerce_sagepay generates a URL based on order_number pointing to a (commerce_sagepay) callback that treats it as an order_id.

As noted in the above issue this is not just harmlessly inconsistent, it will actually cause failure if your site modifies its order_numbers in any way.

Find attached a patch switching these variables around.

CommentFileSizeAuthor
order_number_to_id.patch3.18 KBkingandy

Comments

kingandy’s picture

kingandy
he/him
commented
Priority: Major » Critical

Upgrading to critical because this WILL break people's sites.

ikos’s picture

ikos commented

Hi Andy,

I think an old issue convinced me to pursue a route of changing things to order_number rather than order_id in 3d secure. However your comments make perfect sense and the patch works for me.

I agree with your assessment of severity and will apply this before we go to RC.

Thanks again!

Rich

ikos’s picture

ikos commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.