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.

Sign up for GitHub

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

Jump to bottom

[TK-04948] Kitsune Proxy URL Utility #373

Merged
merged 5 commits into from Sep 23, 2020
Merged

[TK-04948] Kitsune Proxy URL Utility #373

merged 5 commits into from Sep 23, 2020

Conversation

neonphog
Copy link
Contributor

@neonphog neonphog commented Sep 22, 2020 •
edited

Depends on base PR - #370

Given a base transport address like:

kitsune-quic://1.2.3.4:443?t=EugO96mIgrCph7QMpqJkkI5BPY5GuIP7JcCshnwh8FY

We want a nice, human friendly proxy address that incorporates the above without being long and percent escaped. Like:

kitsune-proxy://VlyCSmL5WRKUTOLmF9wF0oFy5Jqbxy0I9KPeXqB_9Z4/kitsune-quic/h/1.2.3.4/p/443/--?t=EugO96mIgrCph7QMpqJkkI5BPY5GuIP7JcCshnwh8FY

Notes:

  • the TLS certificate digests are built into the urls, so signing off on the address incorporates signing off on your TLS certificate.
  • the utility supports the full URL spec, so we can support any strange transports in the future.

@neonphog neonphog added please-one-approval NO-MERGE (dependency) This PR depends on another - and does not point to "develop". labels Sep 22, 2020
thedavidmeister
thedavidmeister approved these changes Sep 23, 2020
View reviewed changes
Copy link
Contributor

@thedavidmeister thedavidmeister left a comment

Choose a reason for hiding this comment

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

approved assuming it's OK to put passwords in the URL

@maackle maackle self-requested a review September 23, 2020 15:47
maackle
maackle previously requested changes Sep 23, 2020
View reviewed changes
Copy link
Member

@maackle maackle left a comment

Choose a reason for hiding this comment

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

Would be nice to have some basic documentation of what the parts of the URL are, so that reading from_full makes more sense.

crates/kitsune_p2p/proxy/src/proxy_url.rs Outdated Show resolved Hide resolved
Base automatically changed from kitsune-proxy-wire to develop September 23, 2020 16:56
@neonphog neonphog removed the NO-MERGE (dependency) This PR depends on another - and does not point to "develop". label Sep 23, 2020
@neonphog neonphog dismissed maackle’s stale review September 23, 2020 16:59

addressed comments

maackle
maackle approved these changes Sep 23, 2020
View reviewed changes
Copy link
Member

@maackle maackle left a comment

Choose a reason for hiding this comment

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

Nice doc!

crates/kitsune_p2p/proxy/src/proxy_url.rs Outdated Show resolved Hide resolved
@neonphog @maackle
Update crates/kitsune_p2p/proxy/src/proxy_url.rs
e0e7fc6 
Co-authored-by: Michael Dougherty <michael.dougherty@holo.host>
@neonphog neonphog merged commit 4b4521d into develop Sep 23, 2020
@neonphog neonphog deleted the kitsune-proxy-url branch September 23, 2020 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thedavidmeister thedavidmeister thedavidmeister approved these changes

@maackle maackle maackle approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@neonphog @thedavidmeister @maackle