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

Convert timestamp to int before calling Sabre_DAV_Property_GetLastModified #8098

Merged
merged 0 commits into from
Apr 8, 2014

Conversation

PVince81
Copy link
Contributor

@PVince81 PVince81 commented Apr 7, 2014

The constructor of Sabre_DAV_Property_GetLastModified is not able to
create a date from a negative timestamp string.

This fix converts the timestamp to int first before passing it.

Fixes #7921

Please review @DeepDiver1975 @icewind1991

@karlitschek
Copy link
Contributor

👍

@icewind1991
Copy link
Contributor

Makes sense 👍

@scrutinizer-notifier
Copy link

The inspection completed: No new issues

@ghost
Copy link

ghost commented Apr 7, 2014

🚀 Test Passed. 🚀
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/4115/

@DeepDiver1975
Copy link
Member

@PVince81 this is actually a bad fix, because server.php is part of upstream SabreDAV. We have a local copy of this class because of depth infinity support, which is meanwhile merged upstream https://github.com/fruux/sabre-dav/pull/398

@PVince81
Copy link
Contributor Author

PVince81 commented Apr 8, 2014

Ah ok, I didn't know it was a copy. I'll raise an issue upstream then.
Closing.

@PVince81 PVince81 closed this Apr 8, 2014
@PVince81 PVince81 deleted the dav-negativetimestampfix branch April 8, 2014 08:11
@PVince81
Copy link
Contributor Author

PVince81 commented Apr 8, 2014

@DeepDiver1975 hmmm how to proceed ? The upstream code seems to be updated a lot.
I could try if the issue still exists on your "upgrade SabreDAV" branch. If yes, raise an issue upstream.
But we might want to quickfix this on stable6 in a way like I did in this PR.

What do you think ?

@DeepDiver1975
Copy link
Member

But we might want to quickfix this on stable6 in a way like I did in this PR.

for OC6 this PR would be okay

for OC7 I'd like to update SabreDAV - see #6723

If we stick with 1.8.7 we have to patch the copy of server.php.
For 1.9.x we need to get this patch upstream asap.

@PVince81
Copy link
Contributor Author

PVince81 commented Apr 8, 2014

I tried #6723 but the timestamp issue still exists there.
I'll prepare a fix for upstream.
Maybe it's better to fix "node.php" instead to make it always return an int as claimed by the PHPDoc.

@PVince81
Copy link
Contributor Author

PVince81 commented Apr 8, 2014

@DeepDiver1975 I just realized after looking at upstream code that our connector/sabre/node.php is written by us, not upstream. Upstream always seem to return int values in node->getLastModified() but ours doesn't.

I'll fix it there instead, so there is nothing to fix upstream in such case.

@PVince81 PVince81 restored the dav-negativetimestampfix branch April 8, 2014 09:53
@PVince81
Copy link
Contributor Author

PVince81 commented Apr 8, 2014

Branch resurrected, reopening.

@PVince81 PVince81 reopened this Apr 8, 2014
@PVince81 PVince81 merged commit 1e559f7 into master Apr 8, 2014
@PVince81
Copy link
Contributor Author

PVince81 commented Apr 8, 2014

Ok, GitHub is confused, I'll submit a separate PR.

@PVince81
Copy link
Contributor Author

PVince81 commented Apr 8, 2014

Replacement PR is here: #8107

@lock lock bot locked as resolved and limited conversation to collaborators Aug 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DateTime::__construct(): Failed to parse time string (-288095782) at position 7 (7): Unexpected character
5 participants