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

fix CryptoBasicFileAttributes #GH-141 #140

Merged
merged 3 commits into from
Sep 15, 2022

Conversation

revintec
Copy link
Contributor

@revintec revintec commented Sep 9, 2022

the old implementation returns wrong file size for symbolic links, e.g. file symlink after ln -s . symlink should be 1

the old implementation returns wrong file size for symbolic links, e.g. file `symlink` after `ln -s . symlink` should be 1
@CLAassistant
Copy link

CLAassistant commented Sep 9, 2022

CLA assistant check
All committers have signed the CLA.

@overheadhunter
Copy link
Member

overheadhunter commented Sep 9, 2022

Hi, can you please:

  1. Create a bug report claiming that symlink size is incorrect and provide some official source documenting what the correct size of a symlink should be like? E.g. man stat:

    The st_size field gives the size of the file (if it is a regular file or a symbolic link) in bytes. The size of a symbolic link is the length of the pathname it contains, without a terminating null byte.

  2. When "fixing" the symlink size, please don't break the directory size 😉

leave `ciphertextFileType(DIRECTORY)` alone while fixing file size for `ciphertextFileType(SYMLINK)`
@revintec revintec changed the title fix CryptoBasicFileAttributes fix CryptoBasicFileAttributes https://github.com/cryptomator/cryptofs/issues/141 Sep 14, 2022
@revintec revintec changed the title fix CryptoBasicFileAttributes https://github.com/cryptomator/cryptofs/issues/141 fix CryptoBasicFileAttributes #GH-141 Sep 14, 2022
Copy link
Member

@overheadhunter overheadhunter left a comment

Choose a reason for hiding this comment

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

Thanks for creating the corresponding issue. The change looks reasonable, however I would like to ask you to stay with the switch statement, as with the ternary operator we lose compile-time exhaustiveness checks.

Furthermore simply moving SYMLINK from one case to the other introduces a significantly smaller diff.

@overheadhunter overheadhunter linked an issue Sep 14, 2022 that may be closed by this pull request
@overheadhunter overheadhunter merged commit 6200230 into cryptomator:develop Sep 15, 2022
@overheadhunter
Copy link
Member

Thank you very much!

@revintec revintec deleted the patch-1 branch September 16, 2022 17:28
@overheadhunter overheadhunter added this to the 2.4.3 milestone Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cryptomator returns incorrect sizes for symbolic links
3 participants