Skip to content

Conversation

@jvillafanez
Copy link
Member

This is mainly for streams that don't wrap another one, or use a stream not provided by the native fopen function such as smbclient_open

NOTE: This PR targets the release branch.

Description

Provide a way not to provide the source of the stream, otherwise we'd get an annoying log complaining about the missing getSource method in the stream, or we could get real errors if the last stream isn't native.

Related Issue

No opened issue yet.

Motivation and Context

Prevent annoying logs. The flow still works without problems, but the log will keep appearing

How Has This Been Tested?

Manually tested

  1. Setup lastest OC master code (10.12.0rc2 - 61a8c68) with the WND app
  2. Setup an external storage with the WND app
  3. Upload an image to the external storage through ownCloud

The warning log appears.

Note that this PR won't solve the issue by itself. It requires changes in the WND app.
Changes in the app (not core) are expected to be backwards-compatible.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE

Expected logs with this PR:

{"reqId":"3hxA7CSmglRj6RwCDqKJ","level":2,"time":"2023-02-28T09:42:33+00:00","remoteAddr":"10.0.2.27","user":"[email protected]","app":"core","method":"GET","url":"\/remote.php\/dav\/files\/[email protected]\/WindowsNetworkDrive\/foonly1\/kHlkYy0hYhvngkpQHcCnAkt5T28.gif?c=63fdcc8977b4c&x=32&y=32&forceIcon=0&preview=1","message":"Cannot get the underlying stream of class OCA\\windows_network_drive\\lib\\smbwrapper\\SMBStream with uri wnd:\/\/smb:\/\/WIN-EJBVQCVTVFU.mountain.tree.prv\/foobar\/foonly1\/kHlkYy0hYhvngkpQHcCnAkt5T28.gif. The OCA\\windows_network_drive\\lib\\smbwrapper\\SMBStream class should has a \"getSource\" method returning the underlying stream, or false \/ null if it isn't wrapping any other stream"}

As told in the log, the app will provide such method (still to be done)

This is mainly for streams that don't wrap another one, or use
a stream not provided by the native fopen function such as
smbclient_open
@update-docs
Copy link

update-docs bot commented Feb 28, 2023

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@jvillafanez
Copy link
Member Author

This is kind of a bug fix for #40600 , which hasn't been released yet. I don't think we need a changelog entry

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

83.3% 83.3% Coverage
0.0% 0.0% Duplication

@jnweiger jnweiger merged commit e237aaa into release-10.12.0 Feb 28, 2023
@delete-merged-branch delete-merged-branch bot deleted the stream_chunk_size_last_stream branch February 28, 2023 10:44
jnweiger added a commit that referenced this pull request Mar 13, 2023
* prepare 10.12

* update translations

* Allow stream wrappers not to provide the underlying stream (#40659)

This is mainly for streams that don't wrap another one, or use
a stream not provided by the native fopen function such as
smbclient_open

* Fix issue if no version has been created yet (#40661)

* fix link in changelog.

* Revert .htaccess change

* OCM via ScienceMesh

Signed-off-by: Michiel de Jong <[email protected]>

* Get plugin from \OC::$server->query()

* Improve changelog entry

Signed-off-by: Michiel de Jong <[email protected]>

* improve changelog entry

Signed-off-by: Michiel de Jong <[email protected]>

* Whitespace change to re-trigger CI

Signed-off-by: Michiel de Jong <[email protected]>

* Upgrading phpseclib/phpseclib (3.0.18 => 3.0.19)

* changelog for phpseclib 3.0.19

* Add test for the new feature

* revert logic to expose free_space, but keep availableStorage config

* Make sender display name in mail notifications configurable (#40671)

* [tx] updated from transifex

* [tx] updated from transifex

* make getFrom configurable

* fix style

* Adjust unit tests for MailNotificationsTest

* add changelog

* Document remove_sender_display_name system setting

* Add unit test cases for when remove_sender_display_name is set to true

---------

Co-authored-by: ownClouders <[email protected]>
Co-authored-by: Phil Davis <[email protected]>

* removing double to

* prepare 10.12.0 RC3

* Bump final 10.12.0 version in version.php

---------

Signed-off-by: Michiel de Jong <[email protected]>
Co-authored-by: Juergen Weigert <[email protected]>
Co-authored-by: Juan Pablo Villafañez <[email protected]>
Co-authored-by: Michiel de Jong <[email protected]>
Co-authored-by: Piotr Mrówczyński <[email protected]>
Co-authored-by: Pasquale Tripodi <[email protected]>
Co-authored-by: ownClouders <[email protected]>
Co-authored-by: EParzefall <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants