Skip to content

Update mdx.py #161

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.

Sign up for GitHub

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

Merged
leifj merged 1 commit into from
Feb 5, 2019
Merged

Update mdx.py #161

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/pyff/mdx.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
from .i18n import language
from . import samlmd
import six
from cgi import escape
Copy link
Member

@c00kiemon5ter c00kiemon5ter Feb 5, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://docs.python.org/3/library/cgi.html#cgi.escape

Deprecated since version 3.2: This function is unsafe because quote is false by default, and therefore deprecated. Use html.escape() instead.

Maybe, we should use just html.escape

Copy link
Contributor Author

@mrvanes mrvanes Feb 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did, but it is only available from 3.2 and pyFF is still in 2.7 land...

Copy link
Contributor

@leifj leifj Feb 5, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pyFF is 3.x and 2.x - all builds are built on 2.7, 3.5, 3.6, 3.7 - is there a six move we can use?

Copy link
Contributor Author

@mrvanes mrvanes Feb 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe in master now, but our deploy is still a 2.7 venv. I expect many others are, due to legacy. Do you want to break all those installations without warning?

Copy link
Contributor

@leifj leifj Feb 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah sorry. I took your comment to mean that pyFF didn't do 3.x but you meant that pyFF still needs to maintain backwards compat with 2.x.

Copy link
Contributor Author

@mrvanes mrvanes Feb 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know six-fu, please consider this PR a best-effort favor to the community...

Copy link
Member

@c00kiemon5ter c00kiemon5ter Feb 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

import six
if six.PY2:
    from cgi import escape
else:
    from html import escape

Copy link
Contributor Author

@mrvanes mrvanes Feb 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But is this compatible with the quote=True named parameter in the call later for html.escape?

Copy link
Member

@c00kiemon5ter c00kiemon5ter Feb 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, both interfaces are the same. The difference is that the default value for the quote parameter is False for cgi.escpae and True for html.escape. Setting the quote parameter yourself works for both modules ;-)

Copy link
Contributor

@leifj leifj Feb 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine merging this and fixing the six move myself


if six.PY2:
_ = language.ugettext
Expand Down Expand Up @@ -576,8 +577,8 @@ def _d(x, do_split=True):
pdict['search'] = "/search/"
pdict['list'] = "/role/idp.json"
else:
pdict['search'] = "{}.s".format(path)
pdict['list'] = "{}.json".format(path)
pdict['search'] = "{}.s".format(escape(path, quote=True))
pdict['list'] = "{}.json".format(escape(path, quote=True))

pdict['storage'] = "/storage/"
cherrypy.response.headers['Content-Type'] = 'text/html'
Expand Down