Skip to content
This repository was archived by the owner on Dec 19, 2025. It is now read-only.

Refactor GatewayUri#57

Merged
nothingmuch merged 2 commits intopayjoin:mainfrom
nothingmuch:gateway-uri
Mar 9, 2025
Merged

Refactor GatewayUri#57
nothingmuch merged 2 commits intopayjoin:mainfrom
nothingmuch:gateway-uri

Conversation

@nothingmuch
Copy link
Collaborator

Use a non optional Scheme & Authority for representing the internal state, instead of a Uri which may be relative, and may contain a path.

Authority is normalized to have an explicit port, as before.

Resolution of authority to a socket addr is now async, to avoid blocking while waiting for host resolution, in anticipation of gateway opt-in allowing adversary chosen hostnames to be resolved which may be a DoS concern.

Closes #51.

Copy link
Collaborator

@DanGould DanGould left a comment

Choose a reason for hiding this comment

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

I agree, GatewayUri makes far more sense as an abstraction over Scheme and Authority. There were so ways to misconfigure GATEWAY_ORIGIN that would not be caught until runtime, I believe this explicitly covers them with the invalid_uris test.

I'm hoping you can clear up some of my questions about the design, though I don't have any significant objections to approving this as-is.

@nothingmuch nothingmuch force-pushed the gateway-uri branch 2 times, most recently from 0f44f8d to 4f0ba22 Compare March 9, 2025 16:50
Use a non optional Scheme & Authority for representing the internal
state, instead of a Uri which may be relative, and may contain a path.

Authority is normalized to have an explicit port, as before.

Resolution of authority to a socket addr is now async, to avoid blocking
while waiting for host resolution, in anticipation of gateway opt-in
allowing adversary chosen hostnames to be resolved which may be a DoS
concern.

Closes payjoin#51.
Copy link
Collaborator

@DanGould DanGould left a comment

Choose a reason for hiding this comment

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

ACK d428437

Once each request handler has been spawned with its own Arc clone
the interior GatewayUri can just be shared to called functions that
require it by reference.
@nothingmuch
Copy link
Collaborator Author

Sorry for pushing another commit after approval, but this additional one makes more sense here than in the next PR

Copy link
Collaborator

@DanGould DanGould left a comment

Choose a reason for hiding this comment

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

ACK 024bc11

love seeing simplifications like this

@nothingmuch nothingmuch merged commit d14ec3f into payjoin:main Mar 9, 2025
5 checks passed
This was referenced Mar 10, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ws and connect bootstrapping may block

2 participants