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.

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

feat: create outline-fetch tool #45

Merged
merged 7 commits into from
Aug 24, 2023
Merged

feat: create outline-fetch tool #45

merged 7 commits into from
Aug 24, 2023

Conversation

fortuna
Copy link
Contributor

@fortuna fortuna commented Aug 12, 2023

This PR creates a command-line tool to fetch a web page given its URL and the transport config.

It also moves outline-connectivity to x/examples and shares the config code.

Direct fetch:

% go run ./x/examples/outline-fetch/main.go https://ipinfo.io
{
  ...
  "city": "Amsterdam",
  "region": "North Holland",
  "country": "NL",
  ...
}                                  

Using a Shadowsocks server:

% go run ./x/examples/outline-fetch/main.go -transport ss://[redacted]@[redacted]:80/ https://ipinfo.io
{
  ...
  "region": "New Jersey",
  "country": "US",
  "org": "AS14061 DigitalOcean, LLC",
  ...
}

Using a SOCKS5 server:

% go run ./x/examples/outline-fetch/main.go -transport socks5://[redacted]:5703 https://ipinfo.io
{
  ... 
  "city": "Berlin",
  "region": "Berlin",
  "country": "DE",
  ...
}

@fortuna fortuna requested a review from jyyi1 August 12, 2023 16:53
@fortuna fortuna changed the base branch from main to fortuna-socks5 August 12, 2023 17:52
@fortuna fortuna changed the title feature: create outline-fetch tool feat: create outline-fetch tool Aug 13, 2023
transport/split/stream_dialer.go Outdated Show resolved Hide resolved
defer func() {
w.bytesCopied += written
}()
n, err := io.CopyN(w.writer, source, w.splitPoint-w.bytesCopied)
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the purpose of splitting the traffic here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because some bytes may had been copied already from previous writes, so you need to discount them to hit the splitPoint correctly.


// NewStreamDialer creates a client that splits the outgoing strean at byte splitpoint.
func NewStreamDialer(dialer transport.StreamDialer, splitPoint int64) (transport.StreamDialer, error) {
if dialer == nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Also check splitPoint > 0 ?

if err != nil {
return nil, fmt.Errorf("failed to parse access key: %w", err)
}
switch accessKeyURL.Scheme {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall we include this logic into the shared ParseAccessKey? So the function could return different configs:

type Scheme uint

const (
  Shadowsocks Scheme = 1 << iota
  Socks5
  Splitter
)

type Config interface {
}

type SSConfig interface {
  Config
}

type Socks5Config interface {
  Config
}

func ParseAccessKey(accessKey string, acceptedSchemes Scheme) (Scheme, Config) {
}

Also the shared NewOutlineStreamDialer(accessKey string, acceptedSchemes Scheme) may also parse to different types.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The config parser should just output the dialer. No need for introducing intermediate configs.

log.Fatalf("Could not create dialer: %v", err)
}
httpClient := &http.Client{Transport: &http.Transport{DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
return dialer.Dial(ctx, addr)
Copy link
Contributor

Choose a reason for hiding this comment

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

We might need to make sure network == "tcp" here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Base automatically changed from fortuna-socks5 to main August 19, 2023 00:17
Copy link
Contributor Author

@fortuna fortuna left a comment

Choose a reason for hiding this comment

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

I removed the split logic, so it doesn't depend on that PR.

Copy link
Contributor Author

@fortuna fortuna left a comment

Choose a reason for hiding this comment

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

@jyyi1 please take another look. I've moves the examples into a subdirectory, and simplified the connectivity to be able to merge the config logic. I'll restore the extra connectivity information once we have tracing in place, but I don't want to block on that to submit the fetch tool, since it's a nice example and useful for testing strategies.

Copy link
Contributor

@jyyi1 jyyi1 left a comment

Choose a reason for hiding this comment

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

Thanks, LTGM. With some small comments.

x/examples/internal/config/config.go Outdated Show resolved Hide resolved
x/examples/internal/config/config.go Outdated Show resolved Hide resolved
if err != nil {
log.Fatalf("Could not create dialer: %v", err)
}
httpClient := &http.Client{Transport: &http.Transport{DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit can we separate this into multiple lines? It looks a little bit messy. But if go format wraps it into a single line then just leave it as it is now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cleaned up

@fortuna fortuna merged commit ea39bb2 into main Aug 24, 2023
6 checks passed
@fortuna fortuna deleted the fortuna-fetch branch August 24, 2023 13:42
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