Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CRITICAL BUG: Race condition on order placement #403

Closed
alanmillo opened this issue Jan 23, 2016 · 6 comments
Closed

CRITICAL BUG: Race condition on order placement #403

alanmillo opened this issue Jan 23, 2016 · 6 comments
Assignees
Labels
Milestone

Comments

@alanmillo
Copy link

Hi, I've found the following bug in the plugin which allows to place orders even when there are no available items (ignores backorders)

Plugin version: 1.1.9.1
Woocommerce: 2.5.0
Wordpress: 4.4.1
Theme: Zerif Lite version 1.8.3.3

Setup:
Wordpress: Tried on existing site with multiple plugins installed and with completely NEW installation where only woocommerce is installed with your plugin

Product: 1 product with do not allow backorders, allow stock management from product page.
woocomerce setting:
Hold stock for 60 minutes, tried with 5 as well.
Enable stock management set to true
Enable guest checkout

Your plugin:
Use paypal express
Tried on sandbox mode and normal mode
Tried with and without skipping final review page

Reproduce:

  • Open a product page in one browser. Add the whole existing quantity of the product to basket
  • Open the same product page on another browser. Add the whole existing quantity of the product to basket
  • Place both orders with different users
  • You are redirected to Paypal. Pay, and you will return to website
  • Both orders are placed, stock value is negative, even though backorders were NOT allowed.

Tried with default Paypal standard from Woocomerce. There is no problem in this case. The main difference after debugging a bit is that paypal standard creates an order in pending payment state right after adding something into the basket. This allows to check whether they might be backorders. I guess they use the Hold stock setting to check as well.

Please, let me know if you need more information. I will post in the wordpress support without disclosing problem since can be problematic for sire owners and very easy to exploit.

Thanks
Alan

@angelleye angelleye added this to the 1.2 milestone Jan 23, 2016
@angelleye angelleye added the bug label Jan 23, 2016
@angelleye
Copy link
Collaborator

Thanks for providing the detail. We will work to get this resolved in the 1.2.0 update.

@angelleye
Copy link
Collaborator

Looks like might be a duplicate of #389

@angelleye
Copy link
Collaborator

@alanmillo we have merged an adjustment that should resolve this into the release branch. We'll be releasing the update soon, but if you want to grab it early feel free.

@alanmillo
Copy link
Author

alanmillo commented Jul 16, 2017

Hey folks,

Looks like this bug is still around. I'm afraid I was not able to update the website till today since I raised the issue, however, once updated to latest version (1.4.5.1) I can see the issue either, is still around, or there has been some regession,

PP for Woocomerce: 1.4.5.1
Woocomerce: 3.1.1
Wordpress: 4.7

Having a look to the commit with the fix, I'm not entirely sure that the changes will actually prevent the race condition,

public function angelleye_check_cart_items() {
if( WC()->cart->check_cart_items() == false) {
wp_redirect(get_permalink(wc_get_page_id('cart')));
exit();
}
}

Given the code changed a lot since version 1.1.9.1, I no longer can provide the fixed that I manually applied to my codebase (this was the main reason why I did not update to latest), however I do remember that right after checking for cart items (as you do in the above function) I also created an order. Creating the order triggered reducing the stock and a failure if stock went < 0.
Obviously this wasn't very elegant since you will end up with some orders and products on hold if the customers does not end the purchase, however the impact is reduced by automatically cancelling orders on hold after X minutes.

Let me know if I'm wrong here, if not, happy to give a hand on testing.

@angelleye
Copy link
Collaborator

@kcppdevelopers Thoughts on this feedback..??

@iMansoorAliKhan
Copy link

@angelleye : His Thread for reference : https://wordpress.org/support/topic/backorders-regesssion/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants