Skip to content

support for windows#7

Open
sj6219 wants to merge 8 commits into
miquels:masterfrom
sj6219:win
Open

support for windows#7
sj6219 wants to merge 8 commits into
miquels:masterfrom
sj6219:win

Conversation

@sj6219

@sj6219 sj6219 commented Jan 8, 2021

Copy link
Copy Markdown

It was modified to work with Windows OS.

Comment thread src/util.rs Outdated
Comment on lines +155 to +157
tm.to_offset(time::UtcOffset::try_current_local_offset().unwrap())
},
Err(_) => time::OffsetDateTime::unix_epoch().to_offset(time::offset!(UTC)),
Err(_) => time::OffsetDateTime::unix_epoch().to_offset(time::UtcOffset::try_current_local_offset().unwrap()),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

These changes break webdav on Linux. The unwrap() on try_current_local_offset() always fails with error::IndeterminateOffset.

Tested on both an x86_64 PC and a ARMv7 device. Both had a timezone (at least/etc/localtime) specified.

It seems this issue was already discussed in time-rs/time#296 . I only skimmed that, but it seems to have been introduced a few versions before the used time crate here. There were mentions of vulnerabilities related to it and it seems that current version, don't have this method anymore.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I didn't handle the error situation for the machine without the timezone setting.
I modified the unwrap() function to the unwrap_or() function.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It's weird since I had a timezone set on my machines. The chrono crate (not time) picks the timezone up fine.

Commit 6b98eea resolved the issue.

This is likely just due to the particular version of time. If it is updated, this will need to be fixed to the proper way the crate does it anyway. But seems current dependencies don't favour bumping the version.

LinusCDE added a commit to LinusCDE/webdav-handler-rs that referenced this pull request Aug 29, 2022
Detailed to the author of the pr here:
miquels#7 (comment)
@LinusCDE

Copy link
Copy Markdown

That commit is a great fix indeed. Seems to continue working flawlessly on Linux as well now. 👍

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.

2 participants