order: Look up prices on the server, not in the POST request. #2750
The web shop form currently submits not only the amount of selected items to
cgi-bin/weborder.pl in a POST request, but also submits the price.
weborder.pl trusts these prices and uses them to compute the total.
These two commits change this by generating a catalogue.xml containing the prices of all items. weborder.pl then looks up prices in that server-side file instead of accepting arbitrary prices in the user-supplied POST request, thereby guaranteeing that the prices have not been tampered with.
* cgi-bin/weborder.pl: Parse order/catalogue.xml to look up prices for item id. * order/order.xsl: Do not generate hidden form input for price.
One more thing of note: catalogue.xml does not require any translations as it is not intended for presentation on the website. It could just as well be replaced with a database, but I wanted to keep the number of changes to a minimum.
The change looks good to me.
We could consider merging all the item.en.xml into a single file (the contents are not translated so we don't gain anything from having separate files per year), which AFAIU save us the additional step of creating the
catalogue.xml file and make things much more straightforward and understandable.
I'm glad you suggest merging all the item files! I was taken aback by the complexity of processing the items when I considered implementing changes to the order form.
Having all items in the same XML file would simplify processing and unlock further improvements.
While we're at it, could we also merge all the
info.*.xml files (one per language)? Or is there a reason why the year must be encoded in the directory name instead of, say, an XML attribute?
info.*.xml files are a different topic, since they are translated: ideally there we would have a separate file per item, so if the text for a given item is not translated, the fallback to the English text works automatically, and missing translations can easily be found.
So we'd end up with something like:
This obviously requires quite a number of changes, but in the end the result was quite logical, understandable and maintainable.
What do you think?
@rekado Thank you very much for your work on this. I was wondering if theire are any blockers. If yes please let us know and we would do our best to help with them.
@reinhard is there anything I could do to help with this PR?
No due date set.
No dependencies set.
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?