Use new Lantern Cloud domain for domain fronting, ensure appropriate requests are fronted #1191
Conversation
go.mod
Outdated
| @@ -38,7 +38,7 @@ require ( | |||
| github.com/getlantern/eventual v1.0.0 | |||
| github.com/getlantern/eventual/v2 v2.0.2 | |||
| github.com/getlantern/filepersist v0.0.0-20210901195658-ed29a1cb0b7c | |||
| github.com/getlantern/flashlight/v7 v7.6.111 | |||
| github.com/getlantern/flashlight/v7 v7.6.112-0.20240929144745-72a54c5a4e73 | |||
There was a problem hiding this comment.
This includes changes from the unmerged flashlight counterpart: getlantern/flashlight#1416
When I've tested these changes thoroughly, I will merge the flashlight PR in and update this reference.
| Transport: proxied.AsRoundTripper( | ||
| func(req *http.Request) (*http.Response, error) { | ||
| log.Tracef("Pro client processing request to: %v (%v)", req.Host, req.URL.Host) | ||
| return rt.RoundTrip(req) | ||
| }, | ||
| ), |
There was a problem hiding this comment.
This doesn't really do anything. proxied.AsRoundTripper is a utility function which simply converts a function to a http.RoundTripper. The request is not necessarily proxied. IMO proxied.AsRoundTripper is misleading and should never have been added to the public API of the proxied package.
I removed NewHTTPClient because it only adds complexity and reader overhead. Instead of calls like:
client := NewHTTPClient(someTransport, timeout)
It's more straight-forward and readable to say:
client := &http.Client{
Transport: someRoundTripper,
Timeout: timeout,
}
I understand that this is opinionated. Let me know if you'd like me to revert this change.
| "github.com/getlantern/lantern-client/internalsdk/protos" | ||
| ) | ||
|
|
||
| type proxyTransport struct { |
There was a problem hiding this comment.
Not only was this unused, it's name was misleading. It does not actually proxy requests. I believe the misunderstanding may have been rooted in the use of proxied.AsRoundTripper by NewHTTPClient - see my comment about this here.
hwh33
changed the title
|
I'm not planning to add reviewers until I look into the CI failure and do some manual testing. Feel free to take a look and comment if you'd like though! |
|
@hwh33 I fixed the merge conflict on this branch: https://github.com/getlantern/lantern-client/tree/atavism/ios-1625 Assuming it's okay to merge the changes here, but let me know and I'm happy to revert them |
|
Yep, that's great, thank you! I'm still planning to do some manual testing with this work, just haven't had a chance yet. Hopefully this week! |
Resolves https://github.com/getlantern/engineering/issues/1429.
This PR can be reviewed by looking at commits individually - each represents a separate step. Some are cosmetic - I noticed a fair bit of unused or misleading code. I'm happy to back those changes out if preferred.
As these changes affect much of the client's ability to communicate with the back-end, I want to run some thorough tests before merging. Thus I am opening this as a draft. I will mark it ready when I've tested the client.