Client: Directory tree depth limit

Created on 3 Oct 2013  Â·  18Comments  Â·  Source: owncloud/client

Above 50 levels of directories the sync breaks however the sync client does not report an error (RC=0).

If there is a policy on a maximum allowed tree depth then it should be handled somehow properly.


Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

ReadyToTest bug sev2-high

All 18 comments

agreed. More than 50 directories should a) work or b) reported as proper error.
This could be a server issue too of course.

I can reproduce the problem.

It seems that, in the update phase, the subdirectories are not seen.

That's because csync hardcode a maximum depth.
In csync_private.h

/**
 * How deep to scan directories.
 */
#define MAX_DEPTH 50

I beleive the reason is not to be fooled by symbolic link loops.
We could mark files in this depth as IGNORED and report that to the user

Ignoring and reporting this clearly to the users sounds good enough.

Many thanks,

kuba

On Oct 14, 2013, at 3:01 PM, Olivier Goffart [email protected] wrote:

That's because csync hardcode a maximum depth.
In csync_private.h

/**

  • How deep to scan directories.
    */
    #define MAX_DEPTH 50
    I beleive the reason is not to be fooled by symbolic link loops.
    We could mark files in this depth as IGNORED and report that to the user

—
Reply to this email directly or view it on GitHub.

@dragotin Can we bubble this up the the UI easily?

We could set the define to a higher value, ie. 250 maybe. Should we do that?

What is the minimum of the maximum depth of all OSes we support?
Although it gets worse thinking about all the storage backend the server supports.
Maybe the client should not limit at all (e.g. 250) and we rely on server errors if it gets too deep

Does this still exist as an issue?

yes

@pmaier1 @michaelstingl to define wanted behaviour and then lets implement it. In any case, when we reach the limit we should have a defined behaviour and not just break.

Limit 50 seems to work. We should display a error message in the UI.

@ogoffart Milestone?

@ogoffart Could me make this configurable with a environment variable?

For now we think a limit of 50 is fine - at least until we hear something else. Anyway we need proper error handling and docs that explain the reason for such limit (symlink loop limiting). In case the directory depth exceeds the limit, the client should show an error message like "Could not be synced due to exceeded directory depth (50). See ".

What needs to be tested? @SamuAlfageme ? Is there already an error message as @pmaier1 suggested?

I've tested the current behavior: The too-deep directory is not synced and is shown in the "not synced" list of files. It's a silent error, similar to other files being excluded due to hiddenness or by exclude pattern.

Perfect IMO, if anybody thinks it should be more then 50, please raise your voice now! We can test more and up the limit ...

@hodyroff Note that 13705999f731042965bae3caa43b7245e7b369e0 increased the limit to 100 and I've updated the docs accordingly in https://github.com/owncloud/client/commit/096cd348f0d1616576bc84cc3b0e282dd26b671b

Was this page helpful?
0 / 5 - 0 ratings

Related issues

michaelstingl picture michaelstingl  Â·  5Comments

ymakhloufi picture ymakhloufi  Â·  3Comments

tflidd picture tflidd  Â·  3Comments

ogasser picture ogasser  Â·  4Comments

codesmaker picture codesmaker  Â·  4Comments