Skip to content

HttpClient getHeader can return empty string #13418

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
rullzer merged 1 commit into from
Jan 8, 2019
Merged

HttpClient getHeader can return empty string #13418

rullzer merged 1 commit into from
Jan 8, 2019

Conversation

@rullzer
Copy link
Member

@rullzer rullzer commented Jan 7, 2019

Fixes #11999

Signed-off-by: Roeland Jago Douma [email protected]

@rullzer rullzer added 3. to review Waiting for reviews regression labels Jan 7, 2019
@rullzer rullzer added this to the Nextcloud 16 milestone Jan 7, 2019
kesselb
kesselb reviewed Jan 7, 2019
View reviewed changes
@rullzer rullzer force-pushed the fix/11999/headers_can_be_empty branch from b120940 to fe1a46f Compare January 7, 2019 21:39
MorrisJobke
MorrisJobke approved these changes Jan 8, 2019
View reviewed changes
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Makes sense 👍

@MorrisJobke
Copy link
Member

MorrisJobke commented Jan 8, 2019

CI fails:

PHP Fatal error:  Class OC\Http\Client\Response contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (OCP\Http\Client\IResponse::getHeader) in /drone/src/github.com/nextcloud/server/lib/private/Http/Client/Response.php on line 35

@rullzer rullzer force-pushed the fix/11999/headers_can_be_empty branch from fe1a46f to d963ff4 Compare January 8, 2019 12:47
@rullzer rullzer force-pushed the fix/11999/headers_can_be_empty branch from d963ff4 to 08970aa Compare January 8, 2019 13:15
@rullzer rullzer merged commit b35db6c into master Jan 8, 2019
@rullzer rullzer deleted the fix/11999/headers_can_be_empty branch January 8, 2019 14:35
@MorrisJobke
Copy link
Member

MorrisJobke commented Jan 8, 2019

@rullzer Is a backport needed?

@rullzer
Copy link
Member Author

rullzer commented Jan 8, 2019

Yeah since it an otherwise return null which php doesn't like in 7 etc ;)

@rullzer
Copy link
Member Author

rullzer commented Jan 8, 2019

/backport to stable15

@rullzer
Copy link
Member Author

rullzer commented Jan 8, 2019

/backport to stable14

@rullzer
Copy link
Member Author

rullzer commented Jan 8, 2019

/backport to stable13

@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Jan 8, 2019
Merged
@backportbot-nextcloud
Copy link

backportbot-nextcloud bot commented Jan 8, 2019

backport to stable15 in #13425

@backportbot-nextcloud
Copy link

backportbot-nextcloud bot commented Jan 8, 2019

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

@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Jan 8, 2019
Merged
@backportbot-nextcloud
Copy link

backportbot-nextcloud bot commented Jan 8, 2019

backport to stable14 in #13426

@MorrisJobke
Copy link
Member

MorrisJobke commented Jan 8, 2019

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

@rullzer Are you doing this?

@MorrisJobke
Copy link
Member

MorrisJobke commented Jan 18, 2019

@rullzer Are you doing this?

❓

@rullzer
Copy link
Member Author

rullzer commented Jan 18, 2019

Nope. We had a different guzzle on 13. So no need as far as I can tell.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kesselb kesselb kesselb left review comments

@MorrisJobke MorrisJobke MorrisJobke approved these changes

@ChristophWurst ChristophWurst ChristophWurst approved these changes

@icewind1991 icewind1991 Awaiting requested review from icewind1991

Assignees

No one assigned

Labels

3. to review Waiting for reviews regression

Projects

None yet

Milestone

Nextcloud 16

Development

Successfully merging this pull request may close these issues.

5 participants

@rullzer @MorrisJobke @ChristophWurst @kesselb