-
-
Notifications
You must be signed in to change notification settings - Fork 575
resolve TypeError in DNS resolution popup formatting #1353
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
Conversation
Hi @Biaogo ! The var type of connection.dst_port is integer, so in this case is correct checking dst_port == 53. Otherwise it evaluates to False. Did you see any strange behaviour on outbound DNS requests that motivated this change? |
When opening certain applications or using ping in the terminal, the gui will crash abnormally
|
I can confirm that it will crash immediately when I ping any sites or IPs, too.
|
@Biaogo Your commit looks more like a workaround, and it only allows if Although I think the better way is to fix the logic of
|
you are right |
Thanks for looking into this @Biaogo @AngelSherry ! This is a problem of the translation files, where the formatting modifier (%s, %d, ...) is changed from the expected position, for example:
We expect the integer value on the last position, but in some translations we receive it in another place. Instead of casting dst_port to str we could use:
(previous calls above that one should be updated as well). We already had this problem in the past, my fault for not updating all the QC.translate() calls. |
hey @Biaogo ! let me know if you want me to fix this issue, or if you'll have time to take a look at it one more time. |
Fix string formatting error when displaying DNS resolution prompts by using consistent string comparison and formatting for port numbers.