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.

Sign up for GitHub

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

Jump to bottom

Use sabre function directly rather than duplicating it #31758

Merged
merged 3 commits into from
Apr 29, 2022
Merged

Use sabre function directly rather than duplicating it #31758

merged 3 commits into from
Apr 29, 2022

Conversation

come-nc
Copy link
Contributor

@come-nc come-nc commented Mar 30, 2022

Signed-off-by: Côme Chilliet come.chilliet@nextcloud.com

@come-nc come-nc self-assigned this Mar 30, 2022
@come-nc come-nc added this to the Nextcloud 24 milestone Mar 30, 2022
@come-nc come-nc force-pushed the fix/utf8-detection-fix branch from 9c6eab7 to 56eff12 Compare March 30, 2022 13:21
@come-nc
Copy link
Contributor Author

come-nc commented Mar 30, 2022

I don’t get it

@blizzz blizzz mentioned this pull request Mar 31, 2022
Merged
@blizzz blizzz mentioned this pull request Apr 7, 2022
Merged
@come-nc
Copy link
Contributor Author

come-nc commented Apr 11, 2022

@CarlSchwan Do you know how I can import the function from Sabre? It seems the autoloader can’t find it.

@come-nc
Copy link
Contributor Author

come-nc commented Apr 12, 2022

The point is to fix #31212
I opened a bug on sabre side: sabre-io/http#181
Using their function would avoid having to fix it on our side.

We can also fix it on our side and hope it will be enough, I do not know if our code goes through the sabre function or not, if we fix on our side it may fix it in UI but not through dav or something.

@blizzz blizzz mentioned this pull request Apr 13, 2022
Merged
@blizzz blizzz modified the milestones: Nextcloud 24, Nextcloud 25 Apr 21, 2022
@PVince81
Copy link
Member

strange if it didn't work for you because it did here on this other line: https://github.com/nextcloud/server/blob/master/lib/private/AppFramework/Http/Request.php#L754

@PVince81 PVince81 force-pushed the fix/utf8-detection-fix branch from bbcc318 to a2d99eb Compare April 25, 2022 14:06
@PVince81
Copy link
Member

I've updated the psalm baseline for the new entry

@come-nc
Copy link
Contributor Author

come-nc commented Apr 26, 2022

/rebase

come-nc and others added 3 commits April 26, 2022 11:10
@come-nc
Use sabre function directly rather than duplicating it
9132ed8 
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
Trying without the use
de5b7f2 
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@PVince81 @come-nc
Update psalm baseline for Sabre decodePath
9cfc70d 
Signed-off-by: Vincent Petry <vincent@nextcloud.com>
@come-nc come-nc force-pushed the fix/utf8-detection-fix branch from a2d99eb to 9cfc70d Compare April 26, 2022 09:12
@come-nc
Copy link
Contributor Author

come-nc commented Apr 26, 2022

/backport to 24

@come-nc come-nc requested review from a team, PVince81, icewind1991 and blizzz and removed request for a team April 28, 2022 12:42
@blizzz blizzz added bug 3. to review Waiting for reviews labels Apr 29, 2022
@blizzz blizzz merged commit 033d527 into master Apr 29, 2022
@blizzz blizzz deleted the fix/utf8-detection-fix branch April 29, 2022 11:05
@backportbot-nextcloud
Copy link

The backport to 24 failed. Please do this backport manually.

@come-nc come-nc mentioned this pull request May 2, 2022
Merged
@come-nc come-nc mentioned this pull request Jun 9, 2022
Closed
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CarlSchwan CarlSchwan CarlSchwan approved these changes

@blizzz blizzz blizzz approved these changes

@PVince81 PVince81 Awaiting requested review from PVince81

@icewind1991 icewind1991 Awaiting requested review from icewind1991

Assignees

@come-nc come-nc

Labels
3. to review Waiting for reviews bug
Projects
None yet
Milestone
Nextcloud 25
Development

Successfully merging this pull request may close these issues.
4 participants
@come-nc @PVince81 @blizzz @CarlSchwan