Skip to content

file-upload: Correctly handle error responses for HTTP2 #25569

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
Merged
ChristophWurst merged 1 commit into from
Mar 11, 2021
Merged

file-upload: Correctly handle error responses for HTTP2 #25569

ChristophWurst merged 1 commit into from
Mar 11, 2021

Conversation

@JakubOnderka
Copy link
Contributor

@JakubOnderka JakubOnderka commented Feb 10, 2021 •
edited
Loading

For HTTP2, errorThrown variable can be empty string:

When an HTTP error occurs, errorThrown receives the textual portion of the HTTP status, such as "Not Found" or "Internal Server Error." (in HTTP/2 it may instead be an empty string)
– https://api.jquery.com/jquery.ajax/ (jquery/api.jquery.com#1146)

So for Chrome or Safari, that follows HTTP/2 specs, Nextcloud will not parse error message. This makes problem at least for AV plugin – see nextcloud/files_antivirus#119

I would be happy if it is be possible to also apply this patchto version 19.X.

@JakubOnderka JakubOnderka force-pushed the JakubOnderka-patch-1 branch from 8195259 to 296ffc7 Compare March 10, 2021 15:30
@JakubOnderka
Copy link
Contributor Author

JakubOnderka commented Mar 10, 2021

PING

PVince81
PVince81 approved these changes Mar 11, 2021
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.

👍 looks fine

@PVince81 PVince81 added 3. to review Waiting for reviews feature: files labels Mar 11, 2021
@ChristophWurst ChristophWurst merged commit 95cddb8 into master Mar 11, 2021
@ChristophWurst ChristophWurst deleted the JakubOnderka-patch-1 branch March 11, 2021 12:54
@kesselb
Copy link
Contributor

kesselb commented Oct 4, 2021

/backport to stable21

@kesselb
Copy link
Contributor

kesselb commented Oct 4, 2021

/backport to stable20

@backportbot-nextcloud
Copy link

backportbot-nextcloud bot commented Oct 4, 2021

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

@backportbot-nextcloud
Copy link

backportbot-nextcloud bot commented Oct 4, 2021

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

This was referenced Oct 4, 2021
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@PVince81 PVince81 PVince81 approved these changes

@ChristophWurst ChristophWurst ChristophWurst approved these changes

@juliusknorr juliusknorr Awaiting requested review from juliusknorr

@rullzer rullzer Awaiting requested review from rullzer

Assignees

No one assigned

Labels

3. to review Waiting for reviews feature: files

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@JakubOnderka @kesselb @PVince81 @ChristophWurst