zopefoundation / zope.publisher

Map requests from HTTP/WebDAV clients, web browsers, XML-RPC and FTP clients onto Python objects
Other
3 stars 13 forks source link

Redo request decoding #15

Closed jinty closed 5 years ago

jinty commented 8 years ago

It looks to me that the original attempt at porting zope.publisher to python3 didn't take into account that FieldStorage started decoding the input in python 3:

https://hg.python.org/cpython/rev/d864328a74e4

In fact the existing decoding code doesn't work at all if you go outside latin-1.

This leaves zope.publisher in a bad place trying to re-do what cgi.py had already done when running in Python 3. I've attempted to kludge around that, trying to maintain backwards compat as well as not redo the decoding cgi.FieldStorage has already done.

This improves things, but is by no means perfect. It's good enough for me and I don't really have time to resolve this fully. A good start for future work would be more tests of the behaviour of IUserPreferredCharsets under non utf-8 environments.

icemac commented 7 years ago

@jinty I stumbled over your PR from 2016. Is it still valid? Should we get it merged?

jinty commented 7 years ago

On Thu, Sep 28, 2017 at 05:54:26AM -0700, Michael Howitz wrote:

@jinty I stumbled over your PR from 2016. Is it still valid? Should we get it merged?

Well, we use it in production every day as a stopgap to better things while we phase out zope.publisher :)

I don't see how anyone could use zope.publisher under python 3 in a non-ascii environment without something like it.

However my patch is ugly and not really the right way to solve the problem. I would be content with just leaving it as a pull request for anyone who needs it to find/use it.

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/zopefoundation/zope.publisher/pull/15#issuecomment-332827197

-- Brian Sutherland

janwijbrand commented 6 years ago

Just a quick comment (I hope to get back later with an more informed opinion/suggestion): I'm with @jinty on this one: zope.publisher essentially does not work on Python 3. I'll now fork it and see if, at least in our stack, I can get it to work again.

mgedmin commented 5 years ago

OMG when I filed #40 and #41 I was wondering "am I crazy? zope.publisher is totally broken on Python 3! how is everyone else using it?", and I completely forgot about this PR!

mgedmin commented 5 years ago

I've dusted this PR off and prepared a new version with some additional fixes in https://github.com/zopefoundation/zope.publisher/pull/46.