-
-
Notifications
You must be signed in to change notification settings - Fork 171
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
add tests for issue #355 #426
Open
niccokunzmann
wants to merge
6
commits into
main
Choose a base branch
from
NicoHood-escape_fix
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
f2c0d23
add tests for issue #355
niccokunzmann 334b019
add other test case mentioned
niccokunzmann 4a634cf
add assert statements
niccokunzmann 169eedc
correct misspelling
niccokunzmann 2e8430a
Fix #355 url escaping
NicoHood 832a166
make sure that the edge case of an empty quoted string is considered
niccokunzmann File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -259,25 +259,6 @@ def from_ical(cls, st, strict=False): | |
% (param, exc)) | ||
return result | ||
|
||
|
||
def escape_string(val): | ||
# '%{:02X}'.format(i) | ||
return val.replace(r'\,', '%2C').replace(r'\:', '%3A')\ | ||
.replace(r'\;', '%3B').replace(r'\\', '%5C') | ||
|
||
|
||
def unescape_string(val): | ||
return val.replace('%2C', ',').replace('%3A', ':')\ | ||
.replace('%3B', ';').replace('%5C', '\\') | ||
|
||
|
||
def unescape_list_or_string(val): | ||
if isinstance(val, list): | ||
return [unescape_string(s) for s in val] | ||
else: | ||
return unescape_string(val) | ||
|
||
|
||
######################################### | ||
# parsing and generation of content lines | ||
|
||
|
@@ -315,34 +296,68 @@ def from_parts(cls, name, params, values, sorted=True): | |
return cls(f'{name}:{values}') | ||
|
||
def parts(self): | ||
"""Split the content line up into (name, parameters, values) parts. | ||
""" | ||
Split the content line up into (name, parameters, values) parts. | ||
|
||
Example with parameter: | ||
DESCRIPTION;ALTREP="cid:[email protected]":The Fall'98 Wild | ||
|
||
Example without parameters: | ||
DESCRIPTION:The Fall'98 Wild | ||
|
||
https://icalendar.org/iCalendar-RFC-5545/3-2-property-parameters.html | ||
""" | ||
try: | ||
st = escape_string(self) | ||
st = self | ||
name_split = None | ||
value_split = None | ||
in_quotes = False | ||
# Any character can be escaped using a backslash, e.g.: "test\:test" | ||
quote_character = False | ||
for i, ch in enumerate(st): | ||
if not in_quotes: | ||
if ch in ':;' and not name_split: | ||
name_split = i | ||
if ch == ':' and not value_split: | ||
value_split = i | ||
# We can also quote using quotation marks. This ignores any output, until another quote appears. | ||
if ch == '"': | ||
in_quotes = not in_quotes | ||
name = unescape_string(st[:name_split]) | ||
continue | ||
|
||
# Ignore input, as we are currently in quotation mark quotes | ||
if in_quotes: | ||
continue | ||
|
||
# Skip quoted character | ||
if quote_character: | ||
quote_character = False | ||
continue | ||
|
||
# The next character should be ignored | ||
if ch == '\\': | ||
quote_character = True | ||
continue | ||
|
||
# The name ends either after the parameter or value delimiter | ||
if ch in ':;' and not name_split: | ||
name_split = i | ||
|
||
# The value starts after the value delimiter | ||
if ch == ':' and not value_split: | ||
value_split = i | ||
|
||
# Get name | ||
name = st[:name_split] | ||
if not name: | ||
raise ValueError('Key name is required') | ||
validate_token(name) | ||
|
||
# Check if parameters are empty | ||
if not name_split or name_split + 1 == value_split: | ||
raise ValueError('Invalid content line') | ||
|
||
# Get parameters (text between ; and :) | ||
params = Parameters.from_ical(st[name_split + 1: value_split], | ||
strict=self.strict) | ||
params = Parameters( | ||
(unescape_string(key), unescape_list_or_string(value)) | ||
for key, value in iter(params.items()) | ||
) | ||
values = unescape_string(st[value_split + 1:]) | ||
|
||
# Get the value after the : | ||
values = st[value_split + 1:] | ||
return (name, params, values) | ||
except ValueError as exc: | ||
raise ValueError( | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
BEGIN:VEVENT | ||
DTSTAMP:20220822T164528Z | ||
UID:[email protected] | ||
DTSTART;TZID=Europe/Berlin:20220822T120000 | ||
DTEND;TZID=Europe/Berlin:20220822T120000 | ||
SUMMARY:test | ||
DESCRIPTION:https://www.facebook.com/events/756119502186737/?acontext=%7B%22source%22%3A5%2C%22action_history%22%3A[%7B%22surface%22%3A%22page%22%2C%22mechanism%22%3A%22main_list%22%2C%22extra_data%22%3A%22%5C%22[]%5C%22%22%7D]%2C%22has_source%22%3Atrue%7D | ||
END:VEVENT |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
BEGIN:VEVENT | ||
DTSTAMP:20220822T164528Z | ||
UID:[email protected] | ||
DTSTART;TZID=Europe/Berlin:20220822T120000 | ||
DTEND;TZID=Europe/Berlin:20220822T120000 | ||
SUMMARY:test | ||
DESCRIPTION:https://www.facebook.com/events/756119502186737/?acontext=%7B%22source%22%3A5%2C%22action_history%22%3A[%7B%22surface%22%3A%22page%22%2C%22mechanism%22%3A%22main_list%22%2C%22extra_data%22%3A%22%5C%22[]%5C%22%22%7D]%2C%22has_source%22%3Atrue%7D | ||
END:VEVENT |
3 changes: 3 additions & 0 deletions
3
src/icalendar/tests/events/issue_355_url_escaping_empty_param.ics
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
BEGIN:VEVENT | ||
ORGANIZER;CN="":that | ||
END:VEVENT |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
"""Tests for Issue 355. | ||
|
||
see https://github.com/collective/icalendar/issues/355 | ||
""" | ||
|
||
def test_facebook_link_is_correctly_parsed(events): | ||
"""The facebook link must not be damaged. | ||
|
||
see https://github.com/collective/icalendar/pull/356#issuecomment-1222626128 | ||
""" | ||
assert events.issue_355_url_escaping["DESCRIPTION"] == "https://www.facebook.com/events/756119502186737/?acontext=%7B%22source%22%3A5%2C%22action_history%22%3A[%7B%22surface%22%3A%22page%22%2C%22mechanism%22%3A%22main_list%22%2C%22extra_data%22%3A%22%5C%22[]%5C%22%22%7D]%2C%22has_source%22%3Atrue%7D" | ||
|
||
def test_other_facebook_link_is_correctly_parsed(events): | ||
niccokunzmann marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"""The facebook link must not be damaged. | ||
|
||
see https://github.com/collective/icalendar/pull/356#issuecomment-1265872696 | ||
""" | ||
expected_result = 'https://www.facebook.com/events/756119502186737/?acontext=%7B%22source%22%3A5%2C%22action_history%22%3A[%7B%22surface%22%3A%22page%22%2C%22mechanism%22%3A%22main_list%22%2C%22extra_data%22%3A%22%5C%22[]%5C%22%22%7D]%2C%22has_source%22%3Atrue%7D' | ||
assert events.issue_355_url_escaping_2["DESCRIPTION"] == expected_result | ||
|
||
def test_empty_quotes(events): | ||
"""Make sure that empty quoted parameter values are supported.""" | ||
assert events.issue_355_url_escaping_empty_param['ORGANIZER'].params['CN'] == "" |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why was this file added? It seems completely identical to your test file