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

implement support for _redirects file #4

Closed
wants to merge 3 commits into from

Conversation

cbrake
Copy link
Collaborator

@cbrake cbrake commented Dec 15, 2021

TODO/Open:

  • - implement basic redirects and simple string compare matcher
  • - implement rewrite for response code 200
  • - support wildcards in matcher
  • - where in flow should _redirects loading go -- before looking for files or after?
  • - should _redirects be somehow cached so we don't need to continually load it?
  • - come up with list of all the URL types/cases/situations we want to handle and write tests

@cbrake cbrake marked this pull request as draft December 15, 2021 02:52
@cbrake cbrake changed the base branch from master to allow-webui-via-ipns December 15, 2021 02:56
@cbrake cbrake changed the base branch from allow-webui-via-ipns to master December 15, 2021 02:56
@cbrake cbrake requested a review from bmann December 15, 2021 03:17
@cbrake
Copy link
Collaborator Author

cbrake commented Dec 15, 2021

@bmann I started a branch for my 404 experiments -- most of this will be rebased or thrown away. I also created a wrapper repo where I plan to document stuff more plus add other files, etc.

https://github.com/cbrake/ipfs-experiments

I can move this to fission space if you'd rather.

I don't have the re-direct working yet, but closer.

@bmann
Copy link
Member

bmann commented Dec 15, 2021

Might be more useful to merge into a branch here rather than master, to make it easier to submit as an upstream PR?

do you want @walkah to do a review?

If you want some testing, I would need a binary and/or instructions on how to build and run locally.

you can always use https://talk.fission.codes for documentation — whatever works for you.

@cbrake
Copy link
Collaborator Author

cbrake commented Dec 15, 2021

@bmann its currently set to merge to master here (vs upstream) -- purely for the convenience of seeing changes. Once I get something that may go upstream, I agree that should merge to another branch, or be another branch.

Not ready for review yet, but will let you know when I am.

@cbrake
Copy link
Collaborator Author

cbrake commented Jan 11, 2022

@bmann function implemented to find _redirects file and print the contents. Possible next steps:

  • parse redirects file
  • implement actual redirect

This is still experimental code and will be re-based/cleaned up (remove instrumentation, etc) after it is working.

A question on processing redirects ... do redirects have priority over files that actually exist, or should we try to resolve the path first, and then load the redirects if it does not? (this determines where in code path redirect processing goes)

@cbrake
Copy link
Collaborator Author

cbrake commented Jan 11, 2022

using https://github.com/cbrake/ipfs-experiments/blob/main/_redirects, I get the following when running this code:

CLIFF: getOrHeadHandler
CLIFF: parsedPath:  /ipfs/bafybeigr6rr5dieoccu74ipzg6bg5cxv4thzz5aevr2wyxfsbjbbp7wzcq/
CLIFF: found redirects file:  /ipfs/bafybeigr6rr5dieoccu74ipzg6bg5cxv4thzz5aevr2wyxfsbjbbp7wzcq/_redirects
CLIFF: redirects file contents: 
 test / 301

CLIFF: resolvedPath:  /ipfs/bafybeigr6rr5dieoccu74ipzg6bg5cxv4thzz5aevr2wyxfsbjbbp7wzcq/
CLIFF: is directory:  true

@cbrake
Copy link
Collaborator Author

cbrake commented Jan 12, 2022

basic support for a _redirects file with a simple matcher has been implemented. I've also rebased/squashed the commits to remove debug messages, so I think this is ready for basic testing.

@cbrake
Copy link
Collaborator Author

cbrake commented Jan 12, 2022

using test setup documented: https://github.com/cbrake/ipfs-experiments, the following redirects work when browsing test and hi:

image

image

@cbrake cbrake changed the title Cbrake/404 experiments implement support for _redirects file Jan 12, 2022
@justindotpub
Copy link

👋🏻 @cbrake. Thanks for getting this work started! Out of curiosity, are you still actively working on this issue? I'm starting to focus on this over here at Fission and would love to collaborate if you're up for it. I'm pretty new to IPFS so I'm trying to get up to speed first. Anyway, if by chance you were wanting to bounce ideas off another human feel free to reach out. 🙏🏻

@cbrake
Copy link
Collaborator Author

cbrake commented Mar 21, 2022

@justincjohnson No, I'm not actively working on this. Let me know if you have any questions -- glad to share anything I know. My github notifications are pretty noisy, so feel free to email me directly if I don't respond here.

@justindotpub
Copy link

I've rebased these changes onto the latest upstream changes in ipfs#8816. All tests out fine per your ipfs-experiments notes @cbrake.

I'm going to continue the work in the new PR / branch, so this PR can be closed now. Thanks for your help on this Cliff!

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.

3 participants