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.

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

[stable13] Make sure the log doesn't try to read from PUT if it can't #9694

Merged
merged 1 commit into from
Jun 1, 2018

Conversation

rullzer
Copy link
Member

@rullzer rullzer commented May 30, 2018

Backport of #9692

Deployed on our instance and fixes the issue

If a PUT request comes in that is not JSON or from encoded. Then we can
only read it (exactly) once. If that is the case we must assume no
shared secret is set.

If we don't then we either are the first to read it, thus causing the
real read of the data to fail.

Or we are later and then it throws an exception (also failing the
request).

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzer rullzer added bug 3. to review Waiting for reviews labels May 30, 2018
@rullzer rullzer added this to the Nextcloud 13.0.3 milestone May 30, 2018
@codecov
Copy link

codecov bot commented May 30, 2018

Codecov Report

Merging #9694 into stable13 will decrease coverage by <.01%.
The diff coverage is 0%.

@@              Coverage Diff               @@
##             stable13    #9694      +/-   ##
==============================================
- Coverage       51.43%   51.43%   -0.01%     
- Complexity      25106    25109       +3     
==============================================
  Files            1612     1612              
  Lines           95544    95549       +5     
  Branches         1376     1376              
==============================================
  Hits            49143    49143              
- Misses          46401    46406       +5
Impacted Files Coverage Δ Complexity Δ
lib/private/Log.php 75% <0%> (-4.13%) 42 <0> (+3)

@MorrisJobke MorrisJobke mentioned this pull request May 31, 2018
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.

Code makes sense 👍

@MorrisJobke MorrisJobke requested a review from skjnldsv June 1, 2018 10:18
@MorrisJobke MorrisJobke merged commit cf4ff77 into stable13 Jun 1, 2018
@MorrisJobke MorrisJobke deleted the backport/9692/stable13 branch June 1, 2018 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants