-
Notifications
You must be signed in to change notification settings - Fork 5
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
Scrub HTTP response headers #5297
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks 👍 , but please refrain from monkey patching if not absolutely necessary. One more 🔁 if you see another way there.
"""We do not use the Bobo exception headers for anything.""" | ||
|
||
def __call__(self): | ||
def _setBCIHeaders(self, t, tb): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm quite unhappy about the continuous use of monkey patches 😞 . we should use them as a last resort only. You might have already considered this but just to be sure, is there an event-handler you could register instead of patching and then scrub the headers again? Maybe IPubFailure
would work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed, apparently there is no other way.
And for reference, pushing for it to get removed upstream for Zope 4. |
I've dropped the added test for the now-reverted rerisen error body scrubbing. Ready for rereview. |
ftw.testbrowser
to allow for not following redirects to test 302 response headers and bodiesZPublisher.interfaces.IPubEnd
to scrub the server version stringOriginal implementation of setting Bobo Call Interface headers onto responses:
https://github.com/zopefoundation/Zope/blob/d916c812bdaf518053b0c3cb2cb3545ff73bc288/src/ZPublisher/HTTPResponse.py#L758-L787
Closes #5185