From e9389f1ad16aac07b48c3f503420b91a4137dce4 Mon Sep 17 00:00:00 2001 From: Vincent Pelletier <vincent@nexedi.com> Date: Thu, 31 Jan 2019 03:47:24 +0100 Subject: [PATCH] erp5_hal_json_style: Fix Base_redirect semantics. As noted in a comment in this BT's Base_redirect implementation, original code does raise when abort_transaction is true. Not raising in this implementation means that this script will return to caller, while it never does on original code. Also, to add insult to injury, this utterly bogus implementation interferes with transaction boundaries. So suddenly, a single publication spans over 2 transactions, which can lead to: - ZODB Connection sharing, breaking transaction isolation - the second transaction implicitly created by this abort (actually, by the next transactional connector registration to transaction) may be committed, in which case anything done after Base_redirect returns will be persistently committed, against caller's explicitly specified intent, and against all developer expectations. NEVER TOUCH TRANSACTION ! Only CMFActivity and unittests are allowed this level of access (and CMFActivity should be modified out of this exceptional state). --- .../extension.erp5.HalStyle.py | 4 --- .../extension.erp5.HalStyle.xml | 5 +++- .../erp5_hal_json_style/Base_redirect.py | 14 +++++----- .../Portal_abortTransaction.xml | 28 ------------------- 4 files changed, 11 insertions(+), 40 deletions(-) delete mode 100644 bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Portal_abortTransaction.xml diff --git a/bt5/erp5_hal_json_style/ExtensionTemplateItem/portal_components/extension.erp5.HalStyle.py b/bt5/erp5_hal_json_style/ExtensionTemplateItem/portal_components/extension.erp5.HalStyle.py index 3c2e748286..15013f5939 100644 --- a/bt5/erp5_hal_json_style/ExtensionTemplateItem/portal_components/extension.erp5.HalStyle.py +++ b/bt5/erp5_hal_json_style/ExtensionTemplateItem/portal_components/extension.erp5.HalStyle.py @@ -14,10 +14,6 @@ def Base_aqBase(self): def Base_aqInner(self): return aq_inner(self) -def Portal_abortTransaction(self): - import transaction - transaction.abort() - def Field_getSubFieldKeyDict(self, field, field_id, key=None): """XXX""" return field.generate_subfield_key(field_id, key=key) diff --git a/bt5/erp5_hal_json_style/ExtensionTemplateItem/portal_components/extension.erp5.HalStyle.xml b/bt5/erp5_hal_json_style/ExtensionTemplateItem/portal_components/extension.erp5.HalStyle.xml index a0fc84e2eb..326a5829e8 100644 --- a/bt5/erp5_hal_json_style/ExtensionTemplateItem/portal_components/extension.erp5.HalStyle.xml +++ b/bt5/erp5_hal_json_style/ExtensionTemplateItem/portal_components/extension.erp5.HalStyle.xml @@ -45,7 +45,10 @@ <item> <key> <string>text_content_warning_message</string> </key> <value> - <tuple/> + <tuple> + <string>W: 71, 4: No exception type(s) specified (bare-except)</string> + <string>W: 5, 0: Unused ZSQLBrain imported from Products.ZSQLCatalog.zsqlbrain (unused-import)</string> + </tuple> </value> </item> <item> diff --git a/bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Base_redirect.py b/bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Base_redirect.py index 3814e7a632..64f61b199a 100644 --- a/bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Base_redirect.py +++ b/bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Base_redirect.py @@ -3,14 +3,10 @@ :param keep_items: is used mainly to pass "portal_status_message" to be showed to the user the new UI supports "portal_status_level" with values "success" or "error" """ +from zExceptions import Redirect from ZTUtils import make_query import json -if abort_transaction: - # Old UI simply throws a Redirect exception and Published does its job - # but we cannot use it here so we abort using External Method - context.getPortalObject().Portal_abortTransaction() - request_form = context.REQUEST.form request_form.update(kw) request_form = context.ERP5Site_filterParameterList(request_form) @@ -34,7 +30,7 @@ if len(parameters): response = context.REQUEST.RESPONSE context.Base_prepareCorsResponse(RESPONSE=response) # http://en.wikipedia.org/wiki/Post/Redirect/Get -response.setStatus(201) +response.setStatus(201, lock=True) response.setHeader("X-Location", "urn:jio:get:%s" % context.getRelativeUrl()) # be explicit with the reponse content type because in case of reports - they # can be in text/plain, application/pdf ... so the RJS form needs to know what @@ -59,4 +55,8 @@ result_dict = { } } } -return json.dumps(result_dict, indent=2) +result = json.dumps(result_dict, indent=2) +if abort_transaction: + response.write(result) + raise Redirect +return result diff --git a/bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Portal_abortTransaction.xml b/bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Portal_abortTransaction.xml deleted file mode 100644 index 07dfbb52a2..0000000000 --- a/bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Portal_abortTransaction.xml +++ /dev/null @@ -1,28 +0,0 @@ -<?xml version="1.0"?> -<ZopeData> - <record id="1" aka="AAAAAAAAAAE="> - <pickle> - <global name="ExternalMethod" module="Products.ExternalMethod.ExternalMethod"/> - </pickle> - <pickle> - <dictionary> - <item> - <key> <string>_function</string> </key> - <value> <string>Portal_abortTransaction</string> </value> - </item> - <item> - <key> <string>_module</string> </key> - <value> <string>HalStyle</string> </value> - </item> - <item> - <key> <string>id</string> </key> - <value> <string>Portal_abortTransaction</string> </value> - </item> - <item> - <key> <string>title</string> </key> - <value> <string>Abort Current Transaction</string> </value> - </item> - </dictionary> - </pickle> - </record> -</ZopeData> -- 2.30.9