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

Fix dash un-escaping to be applied unconditionally #342

Merged
merged 1 commit into from
Sep 8, 2020

Conversation

mgorny
Copy link
Contributor

@mgorny mgorny commented Aug 26, 2020

According to RFC 4880, 'an implementation MAY dash-escape any line,
SHOULD dash-escape lines commencing "From" followed by a space [...]'.
Therefore it is necessary to unescape all lines starting with dash-space
sequences, and not just these that have a dash following this sequence.

Fixes #341

@mgorny
Copy link
Contributor Author

mgorny commented Aug 30, 2020

Added a regression test.

Copy link
Collaborator

@J-M0 J-M0 left a comment

Choose a reason for hiding this comment

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

Ok, this looks good, just one change and I can merge it.

tests/test_99_regressions.py Outdated Show resolved Hide resolved
According to RFC 4880, 'an implementation MAY dash-escape any line,
SHOULD dash-escape lines commencing "From" followed by a space [...]'.
Therefore it is necessary to unescape all lines starting with dash-space
sequences, and not just these that have a dash following this sequence.

Fixes SecurityInnovation#341

Signed-off-by: Michał Górny <[email protected]>
@J-M0
Copy link
Collaborator

J-M0 commented Sep 8, 2020

Quick question, does your regression test pass on your machine? When I try to test it in a python prompt locally I can't get it to pass.

@mgorny
Copy link
Contributor Author

mgorny commented Sep 8, 2020

Well, tests apparently tests pass for me and travis.

@J-M0
Copy link
Collaborator

J-M0 commented Sep 8, 2020

Yeah, it passes for me when I run it through tox on my machine, but typing the code into a >>> prompt doesn't work which is strange.

@mgorny
Copy link
Contributor Author

mgorny commented Sep 8, 2020

Are you copy-pasting it from GitHub's UI? When I do that, I get:

UnicodeEncodeError: 'latin-1' codec can't encode character '\ufffc' in position 35: ordinal not in range(256)

Apprently copy-pasting from the actual file works. It seems that github is inserting some weird unicode characters.

@J-M0
Copy link
Collaborator

J-M0 commented Sep 8, 2020

Yep, that was it. Friggin unicode.

@J-M0 J-M0 changed the title Fix dash escaping to be applied unconditionally Fix dash un-escaping to be applied unconditionally Sep 8, 2020
@J-M0 J-M0 merged commit 6e77ede into SecurityInnovation:master Sep 8, 2020
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.

Dash escaping is not handled correctly
2 participants