Two bugs of iCal parsing was fixed#424
Two bugs of iCal parsing was fixed#424Ellerbach merged 1 commit intonanoframework:mainfrom Naglfario:fix_icalendar_parsing
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (3)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@dotnet-policy-service agree |
@dotnet-policy-service agree |
Ellerbach
left a comment
Ellerbach
left a comment
There was a problem hiding this comment.
Thanks a lot, looks all good. Great you added tests as well.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@Naglfario the pipeline failure are not yours. They're on the hardware device side. |
|
@Naglfario thank you again for your contribution! 🙏😄 .NET nanoFramework is all about community involvement, and no contribution is too small. Please edit it and add an entry with your GitHub username in the appropriate location (names are sorted alphabetically): (Feel free to adjust your name if it's not correct) |
4 participants
Description
DateTimeExtensions.FromiCalendar), before creating a DateTime, the constructor parameters are now validated. Year, month, day, hour, minutes, seconds. If at least one parameter is out of bounds or is not a number, DateTime.MaxValue is returned.DateTimeExtensions.FromiCalendar), when converting a date to UTC, DateTime is now initialized with a ticks count = ticks from the existing date. Previously, date was initialized with zero ticks, and then ticks from existing date were added to it. Previously, this led to an exception, since initialization with zero ticks actually resulted in DateTime.MinValue = 1600-01-01 00:00:00 being created. If you then add ticks from 2025 to such a date, for example, you get a date from 3625 and this value is outside DateTime range since nanoFramework has a limitation, the year of DateTime cannot be >= 3001Motivation and Context
I noticed that when trying to deserialize a string containing many digits (18 or more), I often see exceptions in the debug output console, but this does not interfere with deserialization as such. After investigating, I found that the problem occurs when
JsonConverttries to figure out whether string can be a date in iCal or vCal format.Inside this code, there is an attempt to convert string, if it is numeric, into a date, passing each individual parameter from the year to the seconds as an integer number, but before passing numbers to DateTime constructor, it was not checked before - and whether these numbers can be a year, month, day, and so on. Thus, for example, it could try to create a date with a month = 99.
In addition, there was another bug. It consists in the fact that if at the end of iCal string date there is Z letter, i.e. pointer to time in UTC format, first it creates
MinValuedate withDateTimeKind.UTC, and then ticks from date already obtained at the previous stage were added to created MinValue date. This led to an error, becauseDateTime.MinValueinnanoFramework= 1600-01-01, not a first year, as in the "big .NET"How Has This Been Tested?
Unit tests were written to test the changes, most of which were not passed before the changes were made. After I fixed bugs, I ran the tests again and they passed.
I did not test the changes on real hardware.
I did not run benchmark tests.
Screenshots
Types of changes
Checklist: