Skip to content

DNS: Implement queryStrategy for "localhost" #4303

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

Merged
merged 2 commits into from
Jan 19, 2025
Merged

Conversation

Fangliding
Copy link
Member

#4302
我也不知道为什么当初没实现 或许是有原因的 可能需要考古

@Fangliding Fangliding linked an issue Jan 18, 2025 that may be closed by this pull request
4 tasks
@patterniha
Copy link
Collaborator

patterniha commented Jan 18, 2025

#4302 I don’t know why it wasn’t implemented in the first place. Maybe there’s a reason. Archaeology may be needed.

@cty123 added per server queryStrategy on Sep 18, 2023 #2564
@xiaorouji realized that "udp" was missing on Aug 25, 2024 #3728 , but didn't realize that "localhost" was also missing!!!

and now I accidentally realize that "localhost" was missing :)

@cty123
Copy link
Contributor

cty123 commented Jan 18, 2025

When I implemented the feature for localhost, the ci failed on windows for some reasons. I thought it was windows specific setup problem. Since I hadn't been using a windows machine for a long time, I wasn't able to reproduce the problem locally.

@patterniha
Copy link
Collaborator

#4302 我也不知道为什么当初没实现 或许是有原因的 可能需要考古

your solution for filtering ipv6 and ipv4 is different from @xiaorouji and @cty123 solutions.

Why do we first query for both IPv6 and IPv4 and then filter? we couldn't just query for the IPv4/ipv6 from the beginning?

@Fangliding
Copy link
Member Author

When I implemented the feature for localhost, the ci failed on windows for some reasons. I thought it was windows specific setup problem. Since I hadn't been using a windows machine for a long time, I wasn't able to reproduce the problem locally.

以前test写的有问题就是动不动炸 不一定是代码问题 一般三个平台有一个跑通了就没啥事 全挂了就是代码有问题 现在已经修的七七八八了 只有macos有时候还是莫名其妙炸掉

我再翻了一下 很久以前这个逻辑本来已经在核心里了 把option丢进去重新应用下就好了 根本没必要过滤

@RPRX RPRX changed the title DNS: Implement queryStrategy for localhost DNS DNS: Implement queryStrategy for "localhost" Jan 19, 2025
@RPRX RPRX merged commit f4fd8b8 into main Jan 19, 2025
70 checks passed
@RPRX RPRX deleted the localhost-DNS-queryStrategy branch January 22, 2025 09:36
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.

"localhost" dns always returns both ipv4 and ipv6 regardless of queryStrategy!
4 participants