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

improve gateway symlink handling #6680

Merged
merged 9 commits into from
Jan 6, 2020
Merged

improve gateway symlink handling #6680

merged 9 commits into from
Jan 6, 2020

Conversation

Stebalien
Copy link
Member

  • Correctly calculate symlink sizes.
  • Fix listing of directories with symlinks.
  • Handle files without known sizes.

Note: This doesn't provide actual symlink support. The gateway treats symlinks
like text files.

fixes #6203

@Stebalien
Copy link
Member Author

@senden9, @NatoBoram

func (s *lazySeeker) Seek(offset int64, whence int) (int64, error) {
switch whence {
case io.SeekEnd:
return s.Seek(s.size+offset, io.SeekStart)
Copy link
Contributor

Choose a reason for hiding this comment

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

isn't it s.size-offset ?

Copy link
Member Author

Choose a reason for hiding this comment

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

If we're seeking from the end, the offset will already be negative.

Copy link
Member Author

@Stebalien Stebalien Sep 30, 2019

Choose a reason for hiding this comment

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

Actually, this was wrong. It should have been s.size-1+offset. Nvm. Seeking to the end is correctly done with a Seek(-1, SeekEnd).

@senden9
Copy link

senden9 commented Sep 28, 2019

Build and tested with commit 361bb5f.
Full "symlink support" would be the best but I understand that you want hack it in right now. But I can see now the directory in the gateway web UI without error messages!

@Stebalien Stebalien force-pushed the fix/symlink-size branch 2 times, most recently from 18d1ab2 to 7a8e04e Compare October 1, 2019 18:35
@Stebalien Stebalien requested a review from dirkmc January 3, 2020 22:36
Copy link
Contributor

@dirkmc dirkmc left a comment

Choose a reason for hiding this comment

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

One nit, but otherwise the code LGTM. May be worth adding some tests for the different kinds of Seek()

core/corehttp/lazyseek.go Outdated Show resolved Hide resolved
@Stebalien
Copy link
Member Author

Thanks for catching that (and bugging me to write a test)!

@Stebalien
Copy link
Member Author

(ok, there's definitely a bug here, I'll look into it when I get a chance)

1. Require files to have known sizes. We can add support for unknown sizes
_later_ but we can't use ServeContent for those files.
2. Replace the `sizeReadSeeker` with a `lazySeeker`. This one makes no
assumptions about how it's used so we're less likely to run into weird bugs.
We should be _resolving_ symlinks (sometimes, still need to figure out when to
do this WRT IPNS). However, that's a larger feature.
@Stebalien Stebalien merged commit ca2767a into master Jan 6, 2020
@Stebalien Stebalien deleted the fix/symlink-size branch January 6, 2020 00:54
hacdias pushed a commit to ipfs/boxo that referenced this pull request Jan 27, 2023
improve gateway symlink handling

This commit was moved from ipfs/kubo@ca2767a
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.

internalWebError: operation not supported
4 participants