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

Allow passwords containing colon(s) #2

Merged
merged 3 commits into from
Jan 20, 2014
Merged

Conversation

tflorac
Copy link
Contributor

@tflorac tflorac commented Dec 19, 2013

Hi,
Current implementation doesn't allow passwords containing colon(s): an exception is raised.
Given patch may handle this use case...
Regards,
Thierry

@mgedmin
Copy link
Member

mgedmin commented Dec 20, 2013

👍

However, this patch could use a unit test. And a comment that mentions RFC 2617, which makes it clear usernames aren't allowed to contain a colon, while passwords can have it:

      basic-credentials = base64-user-pass
      base64-user-pass  = <base64 [4] encoding of user-pass,
                       except not limited to 76 char/line>
      user-pass   = userid ":" password
      userid      = *<TEXT excluding ":">
      password    = *TEXT

@mgedmin
Copy link
Member

mgedmin commented Jan 16, 2014

Can I also ask for a mention of this fix in CHANGES.txt?

@tflorac
Copy link
Contributor Author

tflorac commented Jan 16, 2014

Yes, you can, but it's not mandatory!
As you said in your first message, this is a very trivial patch (even if useful for me), so I won't intent a trial if I'm not named in package credits ;-)
Best regards,
Thierry

@mgedmin
Copy link
Member

mgedmin commented Jan 17, 2014

I meant, can you please update the pull request with a CHANGES.txt entry so I can just hit hit the merge button and make a release? ;)

@tflorac
Copy link
Contributor Author

tflorac commented Jan 17, 2014

Content preview: Le 2014-01-17 12:34, Marius Gedminas a écrit? : > I meant,
can you please update the pull request with a CHANGES.txt > entry so I
can just hit hit the merge button and make a release? ;) Oops! Sorry to be
so stupid, I didn't understood your request :-( Of course! I'll do it this
week-end and send you a notice when it's done... [...]

Content analysis details: (-1.0 points, 4.0 required)

pts rule name description


-1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP

Le 2014-01-17 12:34, Marius Gedminas a écrit :

I meant, can you please update the pull request with a CHANGES.txt
entry so I can just hit hit the merge button and make a release? ;)

Oops! Sorry to be so stupid, I didn't understood your request :-(
Of course! I'll do it this week-end and send you a notice when it's
done...

Regards,

Thierry

http://www.imagesdusport.com - http://www.ztfy.org

@tflorac
Copy link
Contributor Author

tflorac commented Jan 18, 2014

Hi Marius,
I added a comment in CHANGES.txt about this patch.
I affected it to the same version (2.0.0a2), as I didn't know if the release number may change or not...
Hope it's OK!

Regards,
Thierry

@mgedmin
Copy link
Member

mgedmin commented Jan 20, 2014

Nah, there's no need to bump the version number -- 2.0.0a2 hasn't been released to PyPI yet.

mgedmin added a commit that referenced this pull request Jan 20, 2014
Allow passwords containing colon(s)
@mgedmin mgedmin merged commit 64ac3b9 into zopefoundation:master Jan 20, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants