Skip to content

http: add absolute request-target support (fixes #193)#651

Open
arnetheduck wants to merge 4 commits into
masterfrom
conn-prov
Open

http: add absolute request-target support (fixes #193)#651
arnetheduck wants to merge 4 commits into
masterfrom
conn-prov

Conversation

@arnetheduck
Copy link
Copy Markdown
Member

Using an absolute request URI and a connection provider that connects to a different server than the http address, generic proxies can be implemented

  • add detail to http connection error
  • fix transport/stream leak when http client construction fails

@arnetheduck
Copy link
Copy Markdown
Member Author

this is an alternative to #618

Comment thread chronos/apps/http/httpclient.nim Outdated
# the tracker so it's safe to spawn here - a bit ugly though,
# would be better to add a "regular" way of doing this
asyncSpawn treader.closeWait()
asyncSpawn twriter.closeWait()
Copy link
Copy Markdown
Collaborator

@cheatfate cheatfate May 29, 2026

Choose a reason for hiding this comment

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

Please do not use asyncSpawn in core code.

  await allFutures(treader.closeWait(), twriter.closeWait())

allFutures() is also NOP here if treader.closeWait() and twriter.closeWait() will return finished Futures.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

new is not async so that can't be done easily .. probably the easiest thing to do is to remove the TLSStreamInitError entirely, or change how asyncstreams are created and closed so that they don't need closing if they haven't been used but both changes seem out of scope here .. can also remove the fix entirely and let them leak but this TODO seemed like a reasonable compromise.

Comment thread chronos/apps/http/httpserver.nim Outdated
Comment thread chronos/apps/http/httpserver.nim Outdated
Comment thread chronos/apps/http/httpclient.nim Outdated
Comment thread chronos/apps/http/httpclient.nim Outdated
arnetheduck and others added 4 commits June 1, 2026 18:50
Using an absolute request URI and a connection provider that connects to
a different server than the http address, generic proxies can be
implemented

* add detail to http connection error
* fix transport/stream leak when http client construction fails
Co-authored-by: Jacek Sieka <arnetheduck@gmail.com>
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.

2 participants