Fix timestamp detection on external FTP #33203
Closed
Conversation
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
|
Hi, can you provide some details? ;) |
|
I can rollback to the original FTP.php and provide some additional logs if you need it. The solution worked great after the modification on the line 358. The error was something like “Call to a member function getTimestamp() on bool”. |
|
Hey @guijusto 👋 Thanks. I think it's good to know what kind of date is the problem here. Index: apps/files_external/lib/Lib/Storage/FTP.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/apps/files_external/lib/Lib/Storage/FTP.php b/apps/files_external/lib/Lib/Storage/FTP.php
--- a/apps/files_external/lib/Lib/Storage/FTP.php (revision c20d34b2caf6ef030ef52e87311ac96d3fa46931)
+++ b/apps/files_external/lib/Lib/Storage/FTP.php (date 1657660464136)
@@ -355,8 +355,13 @@
$data = [];
$data['mimetype'] = $isDir ? FileInfo::MIMETYPE_FOLDER : $mimeTypeDetector->detectPath($name);
- $data['mtime'] = \DateTime::createFromFormat('YmdGis', $file['modify'])->getTimestamp();
- if ($data['mtime'] === false) {
+ $modifyDate = \DateTime::createFromFormat('YmdGis', $file['modify']);
+ if ($modifyDate instanceof \DateTime) {
+ $data['mtime'] = $modifyDate->getTimestamp();
+ } else {
+ /** @var \Psr\Log\LoggerInterface $logger */
+ $logger = \OC::$server->get(\Psr\Log\LoggerInterface::class);
+ $logger->warning('Invalid date. File: ' . $name . ', Date: ' . $file['modify']);
$data['mtime'] = time();
}
if ($isDir) {
Can you patch your system. It should then log the actual file and modify date. |
|
I'm sorry @kesselb. I was unable to reproduce the error again. I tried your patch, but there was no errors/logs. Then I tried to use the original FTP.php and got no errors too. |
|
Thanks for trying @guijusto 👍 Index: apps/files_external/lib/Lib/Storage/FTP.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/apps/files_external/lib/Lib/Storage/FTP.php b/apps/files_external/lib/Lib/Storage/FTP.php
--- a/apps/files_external/lib/Lib/Storage/FTP.php (revision c20d34b2caf6ef030ef52e87311ac96d3fa46931)
+++ b/apps/files_external/lib/Lib/Storage/FTP.php (date 1657699113172)
@@ -355,9 +355,11 @@
$data = [];
$data['mimetype'] = $isDir ? FileInfo::MIMETYPE_FOLDER : $mimeTypeDetector->detectPath($name);
- $data['mtime'] = \DateTime::createFromFormat('YmdGis', $file['modify'])->getTimestamp();
- if ($data['mtime'] === false) {
- $data['mtime'] = time();
+ $modifyDate = \DateTime::createFromFormat('YmdGis', $file['modify']);
+ if ($modifyDate === false) {
+ $date['mtime'] = time();
+ } else {
+ $data['mtime'] = $modifyDate->getTimestamp();
}
if ($isDir) {
$data['size'] = -1; //unknown@solracsf would you mind to update your pr like above? Fallback code to handle an invalid date is already here but does not work. strtotime also returns false and the typecast would change it to 0 then. |
|
Done. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
kesselb
reviewed
Jul 15, 2022
kesselb
approved these changes
Jul 15, 2022
kesselb
requested review from
icewind1991,
a team,
ArtificialOwl and
CarlSchwan
and removed request for
a team
|
/backport to stable24 |
|
/backport to stable23 |
|
/backport to stable22 |
|
Do we know what the reported time is that failed to parse? |
Merged
|
However seems like some tests fail @kesselb ? |
kesselb
force-pushed
the
fix-ftp-timestamp
branch
from
f1c7e1b
to
b83b33d
Compare
|
No idea about the S3 related failure 🤷♂️ |
kesselb
force-pushed
the
fix-ftp-timestamp
branch
from
b83b33d
to
127283c
Compare
|
Hello @kesselb I hope this helps. |
kesselb
force-pushed
the
fix-ftp-timestamp
branch
from
127283c
to
34aa816
Compare
|
Hi @guijusto, thank you very much 👍
@icewind1991 20220804233459.925 |
I guess .925 is the fraction in milliseconds. It should be possible to add some checks for fraction in milliseconds / fraction in microseconds to avoid this warning.
Note the additional |
kesselb
force-pushed
the
fix-ftp-timestamp
branch
from
4bd3b77
to
dc9f850
Compare
kesselb
force-pushed
the
fix-ftp-timestamp
branch
2 times, most recently
from
da4d799
to
72de893
Compare
|
Hey @guijusto, I just updated the patch. Can you give it a try? Thanks 👍 |
Context: #31510 Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
kesselb
force-pushed
the
fix-ftp-timestamp
branch
from
72de893
to
0f9d58d
Compare
Hello @kesselb It is FileZilla Server 1.5.1 |
|
Now it is working as expected. Thanks a lot for working on this @kesselb |
Merged
Merged
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Context: #31510
Fix #31510