Skip to content

fix using FSEEK_END with SeekableHttpStream to get file size#33718

Merged
PVince81 merged 1 commit intomasterfrom
seekable-http-fseek-end
Sep 15, 2022
Merged

fix using FSEEK_END with SeekableHttpStream to get file size#33718
PVince81 merged 1 commit intomasterfrom
seekable-http-fseek-end

Conversation

@icewind1991
Copy link
Member

@icewind1991 icewind1991 commented Aug 26, 2022

Delays the reconnect on fseek to allow fseek(0, FSEEK_END) to return successfully.

This is used by some libraries to get the filesize from stream.

Adds tests to ensure that this method can be used for all storage backends.

@icewind1991 icewind1991 added the 3. to review Waiting for reviews label Aug 26, 2022
@icewind1991 icewind1991 added this to the Nextcloud 25 milestone Aug 26, 2022
@icewind1991 icewind1991 mentioned this pull request Aug 26, 2022
Merged
PVince81
PVince81 approved these changes Aug 29, 2022
View reviewed changes
Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍

@szaimen
Copy link
Contributor

szaimen commented Aug 29, 2022

Seems like some tests fail?

@szaimen szaimen added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Aug 29, 2022
come-nc
come-nc reviewed Aug 29, 2022
View reviewed changes
@icewind1991 icewind1991 force-pushed the seekable-http-fseek-end branch from 66a5067 to a5ad9a1 Compare August 29, 2022 14:54
@icewind1991
Copy link
Member Author

icewind1991 commented Aug 29, 2022

Improved the typings around current a bit

@come-nc
Copy link
Contributor

come-nc commented Aug 29, 2022

You need to run composer run cs:fix

come-nc
come-nc reviewed Aug 29, 2022
View reviewed changes
come-nc
come-nc approved these changes Aug 29, 2022
View reviewed changes
Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

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

Apart from codesniffer and psalm fixes this now looks good.

@icewind1991 icewind1991 force-pushed the seekable-http-fseek-end branch from a5ad9a1 to 841e361 Compare August 29, 2022 17:11
@blizzz blizzz mentioned this pull request Aug 30, 2022
Merged
@icewind1991 icewind1991 force-pushed the seekable-http-fseek-end branch from 841e361 to b65b837 Compare August 30, 2022 18:10
@PVince81
Copy link
Member

PVince81 commented Aug 31, 2022

object store azure fails with:

1) Test\Files\ObjectStore\AzureTest::testFseekSize
fseek(): stream does not support seeking

/drone/src/tests/lib/Files/ObjectStore/ObjectStoreTest.php:156

This was referenced Sep 6, 2022
Merged
Merged
@skjnldsv skjnldsv mentioned this pull request Sep 15, 2022
Merged
@icewind1991 icewind1991 force-pushed the seekable-http-fseek-end branch from b65b837 to 255bcd0 Compare September 15, 2022 15:16
@icewind1991
Copy link
Member Author

icewind1991 commented Sep 15, 2022

Disabled the test in azure for now and will look into fixing azure seek support in a separate PR

@PVince81 PVince81 added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels Sep 15, 2022
come-nc
come-nc reviewed Sep 15, 2022
View reviewed changes
@icewind1991
fix using FSEEK_END with SeekableHttpStream to get file size
d8961ed 
Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991 icewind1991 force-pushed the seekable-http-fseek-end branch from 255bcd0 to d8961ed Compare September 15, 2022 15:23
@PVince81 PVince81 merged commit 0023a10 into master Sep 15, 2022
@PVince81 PVince81 deleted the seekable-http-fseek-end branch September 15, 2022 20:02
@kesselb kesselb mentioned this pull request Jun 1, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@come-nc come-nc come-nc approved these changes

@PVince81 PVince81 PVince81 approved these changes

@artonge artonge Awaiting requested review from artonge

@juliusknorr juliusknorr Awaiting requested review from juliusknorr

@CarlSchwan CarlSchwan Awaiting requested review from CarlSchwan

Assignees

No one assigned

Labels

4. to release Ready to be released and/or waiting for tests to finish

Projects

None yet

Milestone

Nextcloud 25

Development

Successfully merging this pull request may close these issues.

4 participants

@icewind1991 @szaimen @come-nc @PVince81