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

api!: make QR code type for proxy not specific to SOCKS5 #5980

Merged
merged 4 commits into from
Sep 21, 2024

Conversation

link2xt
Copy link
Collaborator

@link2xt link2xt commented Sep 18, 2024

No description provided.

@link2xt link2xt force-pushed the link2xt/qr-proxy branch 2 times, most recently from 251f0d7 to ae13f8d Compare September 18, 2024 13:52
@link2xt
Copy link
Collaborator Author

link2xt commented Sep 18, 2024

Second commit makes it possible to scan ss:// QR codes in addition to https://t.me/socks.
We can also add socks5:// QR code support.

http:// and https:// are currently parsed as URL. We can start scanning them as proxy as well, but currently they are a different QR code type that shows this on Android, offering to open URL in a browser:
http_qr
Advantage of this would be that everything that can be set as a proxy is scanned as a QR proxy type.

@link2xt link2xt self-assigned this Sep 18, 2024
Copy link
Member

@r10s r10s left a comment

Choose a reason for hiding this comment

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

lgtm, did not test, however.

once merged, we should file issues to support the new format

Qr::Url { url } => Some(url),
Qr::Text { text } => Some(text),
Qr::WebrtcInstance { domain, .. } => Some(Cow::Borrowed(domain)),
Qr::Proxy { host, port, .. } => Some(Cow::Owned(format!("{host}:{port}"))),
Copy link
Member

Choose a reason for hiding this comment

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

nice! i thought about that format already when doing #5895, however, regarded the idea as a minor because of some lack of understanding of Cow-strings :)

@r10s
Copy link
Member

r10s commented Sep 18, 2024

http:// and https:// are currently parsed as URL. We can start scanning them as proxy as well, but currently they are a different QR code type that shows this on Android, offering to open URL in a browser

i do not understand. URLs in QR codes are often just URLs that the user should open in a browser, the QR code is mostly there to avoid typing.

do you mean that it is common to have QR codes that just contain a URL that should be used as a proxy then? are they detectable by some format (eg. IP address)?

@link2xt
Copy link
Collaborator Author

link2xt commented Sep 18, 2024

http:// and https:// are currently parsed as URL. We can start scanning them as proxy as well, but currently they are a different QR code type that shows this on Android, offering to open URL in a browser

i do not understand. URLs in QR codes are often just URLs that the user should open in a browser, the QR code is mostly there to avoid typing.

do you mean that it is common to have QR codes that just contain a URL that should be used as a proxy then? are they detectable by some format (eg. IP address)?

@adbenitez wants to use QR codes as the primary interface to add proxies into the list in deltachat/deltachat-android#3292
Then pasting an URL manually is still possible, it is just no different from "paste from clipboard" in the QR code scanner.

This could work if all possible proxy URLs are scanned as Proxy type and all proxy URLs can be given to dc_set_config_from_qr() to start using this proxy.

Since users likely use Delta Chat as the QR code scanner app for HTTPS URLs in advertisements and bills, we can still offer "Open" action for Proxy QR codes that have URL starting with "http://" or "https://".

@link2xt
Copy link
Collaborator Author

link2xt commented Sep 18, 2024

Maybe I should put the URL into text2 and drop the "protocol". Proxy QR code is first of all an URL that can be used as a proxy, but nothing prevents offering to open it in a browser.

@link2xt link2xt force-pushed the link2xt/qr-proxy branch 3 times, most recently from 198d107 to 6d9cc35 Compare September 19, 2024 06:50
@link2xt
Copy link
Collaborator Author

link2xt commented Sep 20, 2024

do you mean that it is common to have QR codes that just contain a URL that should be used as a proxy then? are they detectable by some format (eg. IP address)?

I now detect them by not having a query and a path. So URL https://delta.chat/en/blog is definitely not a proxy, but https://delta.chat may be so it is parsed as a proxy. UI may still want to offer opening proxy QR codes in the browser if they start with https:// or http:// e.g. if they are parsed from the main screen and not from the proxy configuration screen.

@link2xt
Copy link
Collaborator Author

link2xt commented Sep 20, 2024

Now QR codes must have a port to be recognized as a proxy, so https://delta.chat is an URL, but https://delta.chat:443 is a proxy.

@link2xt link2xt merged commit 624ae86 into main Sep 21, 2024
37 checks passed
@link2xt link2xt deleted the link2xt/qr-proxy branch September 21, 2024 18:26
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