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

Add proxy options #144

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Add proxy options #144

wants to merge 5 commits into from

Conversation

devkoks
Copy link

@devkoks devkoks commented Mar 31, 2024

Added support using HTTP proxy for API requests
Screenshot_63

Allow enter address with credentials (ex. http://username:password@example.com:3128)

@marboww
Copy link

marboww commented Mar 31, 2024

In general, our launcher loads endlessly, and since this function is in SETTINGS, it’s impossible to get there. Can this be fixed?

@@ -16,7 +17,7 @@ public static class HappyEyeballsHttp
// This is the workaround.
//
// Implementation taken from https://github.com/ppy/osu-framework/pull/4191/files
public static HttpClient CreateHttpClient(bool autoRedirect = true)
public static HttpClient CreateHttpClient(bool autoRedirect = true, String ?proxyURL = null)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public static HttpClient CreateHttpClient(bool autoRedirect = true, String ?proxyURL = null)
public static HttpClient CreateHttpClient(bool autoRedirect = true, string? proxyURL = null)

Formatting nit.

{
WebProxy webProxy = new WebProxy(proxyURL);
Uri proxyURLUri = new Uri(proxyURL);
Log.Debug(proxyURLUri.UserInfo);
Copy link
Member

Choose a reason for hiding this comment

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

Printing password to log is a very bad idea, comment this out.

Comment on lines 58 to 64
<Grid ColumnDefinitions="Auto,*">
<CheckBox VerticalAlignment="Center" Margin="4" IsChecked="{Binding ProxyEnable}">Enable proxy</CheckBox>
<TextBox Grid.Column="1" Watermark="http://example.com:80" Text="{Binding ProxyURL}" />
</Grid>
<TextBlock VerticalAlignment="Center" TextWrapping="Wrap"
Text="Proxy information for connect to hub and auth center."
Margin="8" />
Copy link
Member

Choose a reason for hiding this comment

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

This should communicate that changing proxy settings requires launcher restart.

Comment on lines +31 to +32
WebProxy webProxy = new WebProxy(proxyURL);
Uri proxyURLUri = new Uri(proxyURL);
Copy link
Member

Choose a reason for hiding this comment

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

What happens if the proxy settings are malformed (invalid URL)? Does it cause the launcher to get stuck crashing on startup with broken config?

Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@VasilisThePikachu
Copy link
Member

Hey! Are you still working on this? Would you mind if I took it off your hands?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants