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

Jsoncs3 share manager #4436

Merged
merged 9 commits into from
Sep 5, 2022
Merged

Conversation

butonic
Copy link
Member

@butonic butonic commented Aug 23, 2022

This pr adds the necessary configuration for the jsoncs3 share manager, introduced with cs3org/reva#3148

@update-docs
Copy link

update-docs bot commented Aug 23, 2022

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@butonic butonic marked this pull request as draft August 23, 2022 08:49
@butonic butonic self-assigned this Aug 23, 2022
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
@ownclouders
Copy link
Contributor

ownclouders commented Aug 25, 2022

💥 Acceptance test Core-API-Tests-ocis-storage-2 failed. Further test are cancelled...

@butonic
Copy link
Member Author

butonic commented Aug 25, 2022

constructing shareids with ! and $ as the delimiter does not work, as the share jail uses the shareid
as the opaqueid for a mount point, leading to stringified versions of the resource id like
<sharjailstorageid>$<sharjailspaceid>!<<<storageid>$<spaceid>!<shareid>>>
options:

  1. implement a lookup table for shareid -> , which we could build when reading the spaces
    -> BAD: introduces an unsharded index
    -> GOOD: shareid can be imported as is
  2. use url safe chars as the delimiters
    -> GOOD: sharded ids allow easier routing and sharding
    -> BAD: shareids will change
    -> BAD: very long ids, eg. 184 chars: a0ca6a90-a365-4782-871e-d44447bbc668$a0ca6a90-a365-4782-871e-d44447bbc668!1751ced0-0701-448a-a377-8f8f536f1f8e^353a0793-c4f5-47f0-9609-bfe6027f8716°d850f14b-dc79-45cb-aa33-dd3457d58848
    -> which chars to use? of "!" / "$" / "&" / "'" / "(" / ")" / "*" / "+" / "," / ";" / "=" the first two are already taken
    • "&" is the url param delimiter,
    • "'" / "(" / ")" / "*" need escaping on the cli
    • "+" is url whitespace
    • "=" is used URL parameters param=value
    • which leaves "," / ";"
    • we could use ":" as the shareid does not appear in the protocol or host part

@butonic
Copy link
Member Author

butonic commented Aug 26, 2022

We'll go with : as the delimiter for the ids as it does not appear in hostnames or protocols.

@butonic butonic force-pushed the jsoncs3-share-manager branch from 8583a2e to 1c403ea Compare August 26, 2022 09:07
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
@butonic butonic force-pushed the jsoncs3-share-manager branch from 1c403ea to bba00e1 Compare August 26, 2022 12:19
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

0.0% 0.0% Coverage
41.0% 41.0% Duplication

@butonic butonic marked this pull request as ready for review September 5, 2022 08:13
@C0rby C0rby merged commit c181e1d into owncloud:master Sep 5, 2022
ownclouders pushed a commit that referenced this pull request Sep 5, 2022
Merge: 29f5251 c5bde43
Author: David Christofas <dchristofas@owncloud.com>
Date:   Mon Sep 5 16:16:18 2022 +0200

    Merge pull request #4436 from aduffeck/jsoncs3-share-manager

    Jsoncs3 share manager
ownclouders pushed a commit that referenced this pull request Sep 6, 2022
Merge: 29f5251 c5bde43
Author: David Christofas <dchristofas@owncloud.com>
Date:   Mon Sep 5 16:16:18 2022 +0200

    Merge pull request #4436 from aduffeck/jsoncs3-share-manager

    Jsoncs3 share manager
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants