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

Add fixes for OSS-Fuzz bugs and test cases #581

Merged

Conversation

ennamarie19
Copy link
Collaborator

This PR introduces the fixes to address two bugs discovered by OSS Fuzz and adds two test cases to ensure that they are not reintroduced down the line.

The bugs are as follows:

  1. Type mismatch in vDDDlists.to_ical when given a date-time that resolves to a string and is joined as bytes
  2. Attempting to pop from an empty stack when parsing 'BEGIN' and 'END' in to_ical

@ennamarie19
Copy link
Collaborator Author

Please merge this in first, as it should resolve pipeline errors in the other branch


def test_vdd_list_type_mismatch():
"""If we pass in a string type, we expect it to be converted to bytes"""
vddd_list = vDDDLists([time(hour=6, minute=6, second=6)])
Copy link
Member

@niccokunzmann niccokunzmann Nov 6, 2023

Choose a reason for hiding this comment

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

I am wondering. to_ical() - should it always return a string or bytes? I would assume bytes. Since the library might not have had a refactoring work since Python2.7, I can understand if these get muddled up. I think, we read calendars from bytes... Let's check. I think, this is a quick fix but in the end, we might want to create type annotations and have a common interface. This might be worth creating an issue. I wonder what the other maintainers think.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for pointing that in consistency out!

@niccokunzmann niccokunzmann merged commit 11eb89b into collective:feat/improve-fuzzing-harness Nov 6, 2023
24 checks passed
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.

3 participants