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

Some email addresses are messed up with parse_email=False #60

Closed
ThiefMaster opened this issue Mar 28, 2012 · 7 comments
Closed

Some email addresses are messed up with parse_email=False #60

ThiefMaster opened this issue Mar 28, 2012 · 7 comments
Labels

Comments

@ThiefMaster
Copy link
Contributor

>>> linkify('doesntmatter@foo-bar.de')
u'doesntmatter@foo<a href="http://-bar.de" rel="nofollow">-bar.de</a>'
@jsocol
Copy link
Contributor

jsocol commented Jul 3, 2012

This only occurs with parse_email=False. Hmm. Unfortunately I'm not sure this is fixable, though I can tweaked the URL regex not to match domains that start with -, because I'm fairly sure that's not allowed.

@ThiefMaster
Copy link
Contributor Author

Sounds like a good workaround.

However, why not always parse email but simply do not modify them if parse_email=False. Or replace them with a placeholder hash and restore them later so the URL regex doesn't catch them.

@jsocol
Copy link
Contributor

jsocol commented Jul 9, 2012

That's an interesting idea but I really want to get 1.2 out, so I'm going to bump to 1.3 and see if that's a good path to follow.

@jmulmer
Copy link

jmulmer commented Jun 9, 2014

I have a similar problem when setting parse_email=False. Any email with following the pattern something.tlds@somedomain.something where tlds is bleach.__init__.TLDS displays a broken link.

For example...

john.coop@domain.com

becomes...

<a href="john.coop">john.coop</a>@domain.com

But...

john.coops@domain.com

is fine, since "coops" isn't in bleach.__init__.TLDS

@willkg willkg modified the milestones: v1.6, v2.0 Oct 31, 2016
@willkg willkg removed this from the v2.0 milestone Mar 6, 2017
@willkg
Copy link
Member

willkg commented Mar 6, 2017

I'm going to drop this from the milestone. I think it needs more thinking and I'm not going to get to it for v2.0.

@willkg willkg added this to the v3.0.0 milestone Oct 2, 2018
@willkg
Copy link
Member

willkg commented Oct 2, 2018

I spent some time fiddling with this today. If we always parse out email addresses, but have parse_email dictate whether we turn them into a url or not, then we break urls that have usernames and passwords in them like this test case:

==================================================== FAILURES ====================================================
_________________________________ tests/test_linkify.py::test_link_http_complete _________________________________

    def test_link_http_complete():
>       assert (
            linkify('https://user:pass@ftp.mozilla.org/x/y.exe?a=b&c=d&e#f') ==
            '<a href="https://user:pass@ftp.mozilla.org/x/y.exe?a=b&amp;c=d&amp;e#f" rel="nofollow">'
            'https://user:pass@ftp.mozilla.org/x/y.exe?a=b&amp;c=d&amp;e#f</a>'
        )
E       assert 'https://user...p;c=d&amp;e#f' == '<a href="http...d&amp;e#f</a>'
E         - https://user:pass@ftp.mozilla.org/x/y.exe?a=b&amp;c=d&amp;e#f
E         + <a href="https://user:pass@ftp.mozilla.org/x/y.exe?a=b&amp;c=d&amp;e#f" rel="nofollow">https://user:pass@ftp.mozilla.org/x/y.exe?a=b&amp;c=d&amp;e#f</a>

tests/test_linkify.py:329: AssertionError
====================================== 1 failed, 299 passed in 0.84 seconds ======================================

I don't think we can get both cases to work without a rewrite of some kind, but I'm not sure what that would look like.

I don't know how to fix this and I'm not that interested in spending more time on it. If someone else wants, they should take a stab at it and let us know what they find. Otherwise, I'll probably close it out because it's been open for 6 years and no one seems interested enough to look into it further.

@willkg
Copy link
Member

willkg commented Oct 6, 2023

This is a problem, but this isn't something we're going to fix. Sorry!

@willkg willkg closed this as completed Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants