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

Show warning when invalid user was passed #12175

Closed
wants to merge 1 commit into from

Conversation

PVince81
Copy link
Contributor

Sometimes there are bugs that cause setupFS() to be called for
non-existing users. Instead of failing hard and breaking the instance,
this fix simply logs a warning.

This is mostly a protection against other bugs. This code path was already triggered by three different bugs (from which some were fixed already).

Please review @icewind1991 @schiesbn @Xenopathic

@icewind1991
Copy link
Contributor

👍 makes sense

@RobinMcCorkell
Copy link
Member

LGTM 👍

@PVince81
Copy link
Contributor Author

Tests failing on master, let's wait for them to be fixed.

@LukasReschke
Copy link
Member

That test will be fixed when #12085 is discussed and merged...

@LukasReschke
Copy link
Member

... let skip the test for now ... - creating PR.

Sometimes there are bugs that cause setupFS() to be called for
non-existing users. Instead of failing hard and breaking the instance,
this fix simply logs a warning.
@scrutinizer-notifier
Copy link

The inspection completed: 1 updated code elements

@LukasReschke
Copy link
Member

@owncloud-bot Retest this please.

@MorrisJobke MorrisJobke added this to the 2014-sprint-08-current milestone Nov 18, 2014
@PVince81
Copy link
Contributor Author

Jenkins and force push... I'll submit a separate PR

@PVince81
Copy link
Contributor Author

Superseded by #12290

@PVince81 PVince81 closed this Nov 19, 2014
@PVince81 PVince81 deleted the ext-preventbreakageduetobugs branch November 19, 2014 09:54
@PVince81
Copy link
Contributor Author

@karlitschek @jnfrmarks I'd like to get this backported.

This issue has been reported at least 4 times already, with at least three different causes / code paths:

  1. recovery key issue: fixed by [stable7] Fix file upload to ext storage when recovery key is enabled #12131
  2. LDAP old/disabled user
  3. Possibly another LDAP issue

Even though the root cause might not be critical in itself, the code path it follows leads to this exception, which itself leads to feature breakage. The fix mitigates that problem by logging a warning instead of throwing and exception, making OC work again.

@karlitschek
Copy link
Contributor

Please backport. Thanks

@PVince81
Copy link
Contributor Author

stable7: f64c6c9

@jnfrmarks reproduction steps are same like #12131

@lock lock bot locked as resolved and limited conversation to collaborators Aug 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants