Skip to content

Conversation

@Davidde94
Copy link
Collaborator

@Davidde94 Davidde94 commented Jun 16, 2021

Add a new Proxy type to enable a HTTPClient to send requests via a SOCKSv5 Proxy server.

@Davidde94 Davidde94 marked this pull request as draft June 16, 2021 13:28
@Davidde94 Davidde94 marked this pull request as ready for review June 16, 2021 19:01
Copy link
Collaborator

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

Cool, looking good! Some notes in the diff.

@Davidde94 Davidde94 requested a review from Lukasa June 17, 2021 14:14
@Davidde94
Copy link
Collaborator Author

@swift-server-bot test this please

@Lukasa Lukasa requested a review from PeterAdams-A June 17, 2021 15:28
@Davidde94 Davidde94 requested a review from Lukasa June 17, 2021 16:09
Copy link
Collaborator

@PeterAdams-A PeterAdams-A left a comment

Choose a reason for hiding this comment

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

A few minor comments but looks generally OK to me.

Comment on lines +345 to +346
public struct Authorization: Hashable {
private enum Scheme: Hashable {
Copy link
Member

Choose a reason for hiding this comment

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

Why was this added?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Getting equatable conformance on the internal Proxy Type enum.

Copy link
Collaborator

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

Cool, a few cleanups needed here.

@Davidde94 Davidde94 requested a review from Lukasa June 18, 2021 11:14
Copy link
Collaborator

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

Ok, this LGTM. Let's get a swift-nio-extras release out the door and we can move forward here.

@Lukasa Lukasa added the 🆕 semver/minor Adds new public API. label Jun 18, 2021
@Davidde94
Copy link
Collaborator Author

@swift-server-bot test this please

import NIOFoundationCompat
import NIOHTTP1
import NIOHTTPCompression
import NIOSOCKS
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need this here :)

Copy link
Member

@fabianfett fabianfett left a comment

Choose a reason for hiding this comment

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

Thanks! Looks awesome!

@Davidde94 Davidde94 merged commit 3fd0658 into swift-server:main Jun 18, 2021
@Davidde94 Davidde94 deleted the de/socks-client branch June 18, 2021 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🆕 semver/minor Adds new public API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants