Migrate http1 proxy connect handler #185
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Can one of the admins verify this patch? |
5 similar comments
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
glbrntt
requested changes
Nov 30, 2022
|
@swift-server-bot add to allowlist |
rnro
requested review from
glbrntt and
dnadoba
and removed request for
glbrntt
dnadoba
reviewed
Nov 30, 2022
Lukasa
requested changes
Nov 30, 2022
rnro
force-pushed
the
migrate_HTTP1_proxy_connect_handler
branch
3 times, most recently
from
5b34b99 to
6c4d28a
Compare
dnadoba
reviewed
Dec 2, 2022
Motivation: Moving the HTTP1ProxyConnectHandler into swift-nio-extras will make the code which is generally useful when dealing with HTTP1 proxies available more easily to a wider audience. Modifications: The code and tests are copied over from https://github.com/swift-server/async-http-client/blob/0b5bec741bfcf941e208d937de2ec29affe750a7/Sources/AsyncHTTPClient/ConnectionPool/ChannelHandler/HTTP1ProxyConnectHandler.swift. Result: HTTP1ProxyConnectHandler will be surfaced via the NIOExtras library
- Add missing `context.fireChannelActive()`,`context.fireChannelInactive()` to pass along events down the pipeline. - Make some members private
- include responseHEAD in invalid response errors - remove precondition in favor of error in event of an unexpected proxy response
rnro
force-pushed
the
migrate_HTTP1_proxy_connect_handler
branch
3 times, most recently
from
cdf4d91 to
060c06d
Compare
rnro
force-pushed
the
migrate_HTTP1_proxy_connect_handler
branch
from
060c06d to
a2da342
Compare
Lukasa
reviewed
Dec 5, 2022
glbrntt
reviewed
Dec 5, 2022
- writes issued whilst the CONNECT is ongoing are now buffered rather than triggering a failure - Error is restructured to do away with `Kind` - failure logic is consolidated in `failWithError`
rnro
force-pushed
the
migrate_HTTP1_proxy_connect_handler
branch
from
da3ea0c to
b34607e
Compare
rnro
requested review from
Lukasa and
glbrntt
and removed request for
Lukasa
glbrntt
reviewed
Dec 15, 2022
dnadoba
reviewed
Dec 15, 2022
glbrntt
reviewed
Dec 19, 2022
There was a problem hiding this comment.
I think I'm more or less happy with this once https://github.com/apple/swift-nio-extras/pull/185/files#r1039437675 has been addressed.
HTTP1ProxyConnectHandler now errors rather than fails a precondition if it becomes inactive unexpectedly when in initializing state
Lukasa
requested changes
Jan 4, 2023
rnro
force-pushed
the
migrate_HTTP1_proxy_connect_handler
branch
from
18678a6 to
619f10b
Compare
unbuffering writes now flushes correctly if the buffer is flushed during unbuffering
rnro
force-pushed
the
migrate_HTTP1_proxy_connect_handler
branch
from
619f10b to
63473f9
Compare
Lukasa
reviewed
Jan 11, 2023
to protect against concurrent modification
Lukasa
approved these changes
Jan 12, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation:
Moving the
HTTP1ProxyConnectHandlerintoswift-nio-extraswill make thecode which is generally useful when dealing with HTTP1 proxies available
more easily to a wider audience.
Modifications:
The code and tests are copied over from https://github.com/swift-server/async-http-client/blob/0b5bec741bfcf941e208d937de2ec29affe750a7/Sources/AsyncHTTPClient/ConnectionPool/ChannelHandler/HTTP1ProxyConnectHandler.swift.
The code is slightly modified to allow the specification of arbitrary headers rather than authorisation information.
Result:
HTTP1ProxyConnectHandlerwill be surfaced via theNIOExtraslibrary