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

undo a database transaction results in "internal server error" generated by waitress #964

Closed
jugmac00 opened this issue May 5, 2021 · 12 comments · Fixed by #969
Closed

Comments

@jugmac00
Copy link
Member

jugmac00 commented May 5, 2021

I am on the Zope 5.1.2.

In ZMI, when I go to the Database tab, and hit undo on a transaction, which cannot be undone because of some data was modifed later on, I get an "Internal Server Error" (generated by waitress).

I certainly can go to the log and have a look there, but I would expect to get a proper message in the ZMI.

P.S.: I am not entirely sure, whether the "Zope" project is the correct one, or rather in the ZODB or ZEO project? Please move if appropriate.

Traceback

Traceback (innermost last):

Module ZPublisher.WSGIPublisher, line 173, in transaction_pubevents
Module transaction._manager, line 257, in commit
Module transaction._manager, line 134, in commit
Module transaction._transaction, line 282, in commit
Module transaction._compat, line 49, in reraise
Module transaction._transaction, line 273, in commit
Module transaction._transaction, line 456, in _commitResources
Module transaction._compat, line 49, in reraise
Module transaction._transaction, line 433, in _commitResources
Module ZODB.DB, line 1085, in tpc_vote
Module ZODB.mvccadapter, line 267, in tpc_vote
Module ZEO.ClientStorage, line 752, in tpc_vote
Module ZEO.asyncio.client, line 764, in call
Module ZEO.asyncio.client, line 743, in call
Module ZEO.asyncio.client, line 756, in wait_for_result
Module concurrent.futures._base, line 439, in result
Module concurrent.futures._base, line 388, in __get_result
ZODB.POSException.MultipleUndoErrors: Undo error 0x04b27157: Undo error 0x04b27157: Some data were modified by a later transaction
@icemac
Copy link
Member

icemac commented May 5, 2021

I think this should be handled by Zope. I'd have expected that the error handling pages would have catched this error for a nicer display.

@dataflake
Copy link
Member

Hiding the original exception is done by waitress, not Zope, since allowing tracebacks to show leads to an information disclosure vulnerability. You can use the expose_tracebacks argument to waitress by adding it to your WSGI configuration .ini file in the section for [server:<name>]: expose_tracebacks = True.

expose_tracebacks is documented at the bottom of https://docs.pylonsproject.org/projects/waitress/en/stable/arguments.html and using it to debug Zope with WSGI is documented at https://zope.readthedocs.io/en/latest/operation.html#debugging-zope-applications-under-wsgi.

These configuration options provide a simple traceback display, it's nothing pretty, but at least it doesn't "hide" the real traceback. But again, this is an information disclosure and should only be used for debugging, not in production.

A real debugging tool is part of the werkzeug WSGI server. If you install the shim package dataflake.wsgi.werkzeug (see https://dataflakewsgiwerkzeug.readthedocs.io/en/latest/installation.html) and then add a [server...] section in your WSGI .ini configuration as shown at https://dataflakewsgiwerkzeug.readthedocs.io/en/latest/usage.html#using-the-werkzeug-debugger you will get a browser-based debugging tool. I don't think I have to stress that this is an absolute no-go for any public site.

@icemac
Copy link
Member

icemac commented May 6, 2021

How did Zope 2 behave in this case? I think it rendered an error message on the Undo page instead of a HTTP-500 error, didn't it? (I currently do not have a Zope 2 at hand to check.)

I seem to remember that we fixed this behavior in other places. Maybe we can do it here, too.

@dataflake
Copy link
Member

What you see in the browser when a 5xx HTTP status is set by Zope is influenced by the WSGI server if WSGI is used, not Zope. The default behavior for waitress (and werkzeug) is to hide the error and prevent information disclosure.

I don't recall any "fix" in Zope 4 or 5. I think it's been that way forever after adopting waitress. I think the only way you could show a nicer error message without triggering the "hiding" behavior in waitress would be to register catch-all error views that forced the HTTP status back to 200.

Zope 2 used a default standard_html_error or whatever you put into a ZODB method with that name. Traceback information was passed into that error method and you could choose to display it. I don't recall if the default that gets rendered if there is no standard_html_error in the ZODB showed the traceback. That would have been stupid - information disclosure.

@jugmac00
Copy link
Member Author

jugmac00 commented May 6, 2021

The error message would only be shown to the logged in user in the ZMI, so I do not think there is an information disclosure.

When we switched to waitress, we encountered many uncaught exceptions.

While I do not find a fixed one immediately, there are similar issues e.g. in the ZCatalog:

I am pretty sure we fixed quite a couple of these issues...

I am a bit in a hurry, but maybe this one is a similar thing?
https://github.com/zopefoundation/Products.PythonScripts/pull/23/files

@dataflake
Copy link
Member

dataflake commented May 6, 2021

The error message and traceback would be shown to any anonymous visitor. There is no distinction between logged in or not, or "in the ZMI" or not. So there definitely is an information disclosure. Remember that this behavior affects any site error that's not covered by explicitly defined error views, not just your Undo error.

@jugmac00
Copy link
Member Author

jugmac00 commented May 6, 2021

I still don't get it.

Only users with the undo_changes permission can trigger the function.

Zope/src/App/Undo.py

Lines 100 to 101 in f3bfc64

@security.protected(undo_changes)
def manage_undo_transactions(self, transaction_info=(), REQUEST=None):

Without triggering the function, there can't be a traceback.

Or in other words. When an upriviledged user tries to access the mentioned function, the user gets an unauthorized error.

@d-maurer
Copy link
Contributor

d-maurer commented May 6, 2021 via email

@dataflake
Copy link
Member

I'll summarize:

  • the specific error you refer to, the UndoError, shows the exception handling that is common to all exceptions not handled by Zope itself, it applies to all exceptions for all authenticated or anonymous users.
  • the "Internal Server Error" page is generated by waitress, and it doesn't contain any information for security reasons
  • you can still get the "real" traceback by changing a waitress setting in your WSGI configuration .ini file. You should not do this on a production site, though.
  • other WSGI servers, such as werkzeug, have full debugging facilities in the browser and you can integrate them easily, so this is not something we need to create ourselves.

I had created that dataflake.wsgi.werkzeug shim a couple years ago thinking it would be super useful, but in the end I never used it myself. I agree with Dieter, the traceback information I can see at the console, in the logs or in the optional error_log object has always been sufficient. But I understand that everyone has their own debugging workflow.

@icemac
Copy link
Member

icemac commented May 7, 2021

After a bit of searching I found an issue where we switched from a HTTP-500 to render an error message on the page in the ZMI: #357 (quite the same fix as mentioned by @jugmac00: zopefoundation/Products.PythonScripts#23)
I think a similar behavior could be possible here: It is no internal error, but the user has to select different transactions to undo them all together.

@icemac
Copy link
Member

icemac commented May 7, 2021

I fired up a Zope 2.13 and got this when trying to undo an older transaction:

Bildschirmfoto 2021-05-07 um 08 22 18

This is not what was in my head.
Additionally there is difference to #357 where the exception is risen in the application code. Here it occurs during commit. This makes it a bit harder to catch the exception.

@d-maurer
Copy link
Contributor

d-maurer commented May 7, 2021 via email

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

Successfully merging a pull request may close this issue.

4 participants