Vulnerability in EPiServer.Forms
I am using Commerce 12.10.0 and below is the scenario:
I added an item into the cart and then applied a shipping coupon (Free shipping). Upon applying the coupon I get the error mentioned below (step 2)
1) Added a weight based shipping method: https://www.screencast.com/t/trWQxz7zJ
2) Below is the stack trace for the error: https://www.screencast.com/t/TRpqw9MgFR6D
I also had a look into the reflector based on the stack trace and I see the code mentioned below:
public Money GetDiscountedShippingAmount(IShipment shipment, IMarket market, Mediachase.Commerce.Currency currency)
Validator.ThrowIfNull(nameof (shipment), (object) shipment);
Validator.ThrowIfNull(nameof (market), (object) market);
Money money = this.GetShippingCost(shipment, market, currency) - shipment.GetShipmentDiscountPrice(currency);
Looking at this I feel the issue is where you subtract GetShippingCost with GetShipmentDiscountPrice. And GetShipmentDiscountPrice being the culprit which leads to a negative value and then throws the error.
Right before running the calculators in your own code.
If you do:
What does that give you?
Yep, as I suspected. The dreaded rounding error. :P
The underlying cause is that the PromotionEngine rounds the resulting saved amount before setting it on the Shipment.SavedAmount field.
So either you avoid using shipment costs with more than X number of decimal places (X in your case being 2).
... if you can't do that I'm hard pressed to find a good solution to this.
But technically you can modify to how many decimal places the promotion engine rounds off to. You can try doing this before running the promotion engine. Like so:
var oldCurrencyDecimalDigits = orderGroup.Currency.Format.CurrencyDecimalDigits,
orderGroup.Currency.Format.CurrencyDecimalDigits = 5;
// Run PromotionEngine
orderGroup.Currency.Format.CurrencyDecimalDigits = oldCurrencyDecimalDigits;
This should (I hope, I haven't tested it :D) mean that the PromotionEngine rounds off correctly in your case.
But I'm assuming that this approach will come with other issues since all calculations made based on the ordergroup in the Promotion Engine will be rounded off the same way, which may be incorrect in another case.
@Jafet Valdez: Thank you for the response, I see this as a bug with the promotions engine then. It should not round the saved amount right? I appreciate your suggestion to try and modify the number of decimal places rounding. But since the effect of this change might cause other issues I dont want to move forward with this. Would you suggest to file a bug in this case then? The project I am working on is approaching the go-live date.
I'm not so sure that it can be considered a bug per sé. It's rounding according to the defaults for currencies in C#. Like Episerver Commerce does in most other places.
It needs to round to be able to work with various types of percentages or you'd have other calculation issues instead.
Personally I'd say that doing the same rounding in your ShippingMethod would be the best solution in general, but I understand that this might not be possible for business reasons.
So contact Developer Support to get a path forward on this.
It looks like modifying Currency.Format.CurrencyDecimalDigit is the way to go for now until Episerver gets a fix out in the next major release
Take a look at this blog post:
@Jafet Valdez: Thanks the response. Marking your reply as the answer :)