-
Notifications
You must be signed in to change notification settings - Fork 5
feat: add AliDNS and Cloudflare DNS resolvers with corresponding tests #30
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
Changes from 8 commits
94ceabd
02c4ae1
67ee6a6
8cba2d9
0ab6226
ae5176e
351544b
9885826
d2eb43c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,18 +1,26 @@ | ||
| import { setupServer, SetupServerApi } from "msw/node"; | ||
| import { http, HttpResponse } from "msw"; | ||
| import { CustomDnsResolver, getDocumentStoreRecords, queryDns, parseDocumentStoreResults, getDnsDidRecords } from "."; | ||
| import { | ||
| aliDnsResolver, | ||
| cloudflareDnsResolver, | ||
| getDocumentStoreRecords, | ||
| getDnsDidRecords, | ||
| googleDnsResolver, | ||
| parseDocumentStoreResults, | ||
| queryDns, | ||
| } from "."; | ||
| import { DnsproveStatusCode } from "./common/error"; | ||
|
|
||
| describe("getCertStoreRecords", () => { | ||
| const sampleDnsTextRecordWithDnssec = { | ||
| type: "openatts", | ||
| net: "ethereum", | ||
| netId: "3", | ||
| dnssec: true, | ||
| dnssec: false, | ||
| addr: "0x2f60375e8144e16Adf1979936301D8341D58C36C", | ||
| }; | ||
| test("it should work", async () => { | ||
| const records = await getDocumentStoreRecords("donotuse.openattestation.com"); | ||
| const records = await getDocumentStoreRecords("donotuse.trustvc.io"); | ||
| expect(records).toStrictEqual([sampleDnsTextRecordWithDnssec]); | ||
| }); | ||
|
|
||
|
|
@@ -27,14 +35,14 @@ describe("getCertStoreRecords", () => { | |
|
|
||
| describe("getDnsDidRecords", () => { | ||
| test("it should work", async () => { | ||
| const records = await getDnsDidRecords("donotuse.openattestation.com"); | ||
| const records = await getDnsDidRecords("donotuse.trustvc.io"); | ||
| expect(records).toStrictEqual([ | ||
| { | ||
| type: "openatts", | ||
| algorithm: "dns-did", | ||
| publicKey: "did:ethr:0xE712878f6E8d5d4F9e87E10DA604F9cB564C9a89#controller", | ||
| version: "1.0", | ||
| dnssec: true, | ||
| dnssec: false, | ||
| }, | ||
| ]); | ||
| }); | ||
|
|
@@ -177,29 +185,29 @@ describe("queryDns", () => { | |
| RA: true, | ||
| AD: true, | ||
| CD: false, | ||
| Question: [{ name: "donotuse.openattestation.com.", type: 16 }], | ||
| Question: [{ name: "donotuse.trustvc.io.", type: 16 }], | ||
| Answer: [ | ||
| { | ||
| name: "donotuse.openattestation.com.", | ||
| name: "donotuse.trustvc.io.", | ||
| type: 16, | ||
| TTL: 300, | ||
| data: "openatts a=dns-did; p=did:ethr:0xE712878f6E8d5d4F9e87E10DA604F9cB564C9a89#controller; v=1.0;", | ||
| }, | ||
| { | ||
| name: "donotuse.openattestation.com.", | ||
| name: "donotuse.trustvc.io.", | ||
| type: 16, | ||
| TTL: 300, | ||
| data: | ||
| "openatts DO NOT ADD ANY RECORDS BEYOND THIS AS THIS DOMAIN IS USED FOR DNSPROVE NPM LIBRARY INTEGRATION TESTS", | ||
| }, | ||
| { | ||
| name: "donotuse.openattestation.com.", | ||
| name: "donotuse.trustvc.io.", | ||
| type: 16, | ||
| TTL: 300, | ||
| data: "openatts fooooooobarrrrrrrrr this entry exists to ensure validation works", | ||
| }, | ||
| { | ||
| name: "donotuse.openattestation.com.", | ||
| name: "donotuse.trustvc.io.", | ||
| type: 16, | ||
| TTL: 300, | ||
| data: "openatts net=ethereum netId=3 addr=0x2f60375e8144e16Adf1979936301D8341D58C36C", | ||
|
|
@@ -208,22 +216,7 @@ describe("queryDns", () => { | |
| Comment: "Response from 205.251.199.177.", | ||
| }; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can help update to use
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
| const testDnsResolvers: CustomDnsResolver[] = [ | ||
| async (domain) => { | ||
| const data = await fetch(`https://dns.google/resolve?name=${domain}&type=TXT`, { | ||
| method: "GET", | ||
| }); | ||
|
|
||
| return data.json(); | ||
| }, | ||
| async (domain) => { | ||
| const data = await fetch(`https://cloudflare-dns.com/dns-query?name=${domain}&type=TXT`, { | ||
| method: "GET", | ||
| headers: { accept: "application/dns-json", contentType: "application/json", connection: "keep-alive" }, | ||
| }); | ||
| return data.json(); | ||
| }, | ||
| ]; | ||
| const testDnsResolvers = [googleDnsResolver, cloudflareDnsResolver, aliDnsResolver]; | ||
|
|
||
| afterEach(() => { | ||
| server.close(); | ||
|
|
@@ -239,7 +232,7 @@ describe("queryDns", () => { | |
| server = setupServer(...handlers); | ||
| server.listen(); | ||
|
|
||
| const records = await queryDns("https://donotuse.openattestation.com", testDnsResolvers); | ||
| const records = await queryDns("https://donotuse.trustvc.io", testDnsResolvers); | ||
| const sortedAnswer = records?.Answer.sort((a, b) => a.data.localeCompare(b.data)); | ||
| expect(sortedAnswer).toMatchObject(sampleResponse.Answer); | ||
| }); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this will fail too, since OA removed the url.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
@@ -256,7 +249,28 @@ describe("queryDns", () => { | |
| server = setupServer(...handlers); | ||
| server.listen(); | ||
|
|
||
| const records = await queryDns("https://donotuse.openattestation.com", testDnsResolvers); | ||
| const records = await queryDns("https://donotuse.trustvc.io", testDnsResolvers); | ||
|
|
||
| const sortedAnswer = records?.Answer.sort((a, b) => a.data.localeCompare(b.data)); | ||
| expect(sortedAnswer).toMatchObject(sampleResponse.Answer); | ||
| }); | ||
|
|
||
| test("Should fallback to third dns when first and second dns is down", async () => { | ||
| const handlers = [ | ||
| http.get("https://dns.google/resolve", (_) => { | ||
| return new HttpResponse(null, { status: 500 }); | ||
| }), | ||
| http.get("https://cloudflare-dns.com/dns-query", (_) => { | ||
| return new HttpResponse(null, { status: 500 }); | ||
| }), | ||
| http.get("https://dns.alidns.com/resolve", (_) => { | ||
| return HttpResponse.json(sampleResponse); | ||
| }), | ||
| ]; | ||
| server = setupServer(...handlers); | ||
| server.listen(); | ||
|
|
||
| const records = await queryDns("https://donotuse.trustvc.io", testDnsResolvers); | ||
|
|
||
| const sortedAnswer = records?.Answer.sort((a, b) => a.data.localeCompare(b.data)); | ||
| expect(sortedAnswer).toMatchObject(sampleResponse.Answer); | ||
|
|
@@ -270,14 +284,16 @@ describe("queryDns", () => { | |
| http.get("https://cloudflare-dns.com/dns-query", (_) => { | ||
| return new HttpResponse(null, { status: 500 }); | ||
| }), | ||
| http.get("https://dns.alidns.com/resolve", (_) => { | ||
| return new HttpResponse(null, { status: 500 }); | ||
| }), | ||
| ]; | ||
| server = setupServer(...handlers); | ||
| server.listen(); | ||
| try { | ||
| await queryDns("https://donotuse.openattestation.com", testDnsResolvers); | ||
| } catch (e: any) { | ||
| expect(e.code).toStrictEqual(DnsproveStatusCode.IDNS_QUERY_ERROR_GENERAL); | ||
| } | ||
|
|
||
| await expect(queryDns("https://donotuse.trustvc.io", testDnsResolvers)).rejects.toMatchObject({ | ||
| code: DnsproveStatusCode.IDNS_QUERY_ERROR_GENERAL, | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
|
|
@@ -321,6 +337,13 @@ describe("getDocumentStoreRecords for Astron", () => { | |
| addr: "0x18bc0127Ae33389cD96593a1a612774fD14c0737", | ||
| dnssec: false, | ||
| }, | ||
| { | ||
| type: "openatts", | ||
| net: "ethereum", | ||
| netId: "1338", | ||
| addr: "0x94FD21A026E29E0686583b8be71Cb28a8ca1A8d4", | ||
| dnssec: false, | ||
| }, | ||
| { | ||
| type: "openatts", | ||
| net: "ethereum", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| import type { CustomDnsResolver, IDNSQueryResponse } from "../.."; | ||
|
|
||
| /** Ali DNS JSON API uses numeric RRTYPE; 16 = TXT */ | ||
| const ALI_DNS_TXT_QUERY_TYPE = "16"; | ||
| export const aliDnsResolver: CustomDnsResolver = async (domain) => { | ||
| const url = new URL("https://dns.alidns.com/resolve"); | ||
|
|
||
| if (!domain) { | ||
| throw new Error("Domain is required"); | ||
| } | ||
|
|
||
| url.searchParams.set("name", domain); | ||
| url.searchParams.set("type", ALI_DNS_TXT_QUERY_TYPE); | ||
|
|
||
| const res = await fetch(url); | ||
|
|
||
| if (!res.ok) { | ||
| throw new Error(`Ali DNS request failed: HTTP ${res.status}`); | ||
| } | ||
|
|
||
| let data; | ||
| try { | ||
| data = await res.json(); | ||
| } catch { | ||
| throw new Error("Failed to parse DNS response JSON"); | ||
| } | ||
|
|
||
| return data as IDNSQueryResponse; | ||
| }; |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,29 @@ | ||||||||||||||||||||||||||||||||||
| import type { CustomDnsResolver, IDNSQueryResponse } from "../.."; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| export const cloudflareDnsResolver: CustomDnsResolver = async (domain) => { | ||||||||||||||||||||||||||||||||||
| const url = new URL("https://cloudflare-dns.com/dns-query"); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| if (!domain) { | ||||||||||||||||||||||||||||||||||
| throw new Error("Domain is required"); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| url.searchParams.set("name", domain); | ||||||||||||||||||||||||||||||||||
| url.searchParams.set("type", "TXT"); | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+6
to
+11
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Harden domain validation for whitespace-only input. At Line 6, 🔧 Suggested patch export const cloudflareDnsResolver: CustomDnsResolver = async (domain) => {
const url = new URL("https://cloudflare-dns.com/dns-query");
+ const normalizedDomain = domain?.trim();
- if (!domain) {
+ if (!normalizedDomain) {
throw new Error("Domain is required");
}
- url.searchParams.set("name", domain);
+ url.searchParams.set("name", normalizedDomain);
url.searchParams.set("type", "TXT");📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const res = await fetch(url, { | ||||||||||||||||||||||||||||||||||
| headers: { Accept: "application/dns-json" }, | ||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| if (!res.ok) { | ||||||||||||||||||||||||||||||||||
| throw new Error(`Cloudflare DNS request failed: HTTP ${res.status}`); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| let data; | ||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||
| data = await res.json(); | ||||||||||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||||||||||
| throw new Error("Failed to parse DNS response JSON"); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| return data as IDNSQueryResponse; | ||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These tests are still coupled to mutable public DNS data.
Switching to
donotuse.trustvc.ioand updating the Astron fixture fixes the current failures, but these cases still hit live zones and assert the full TXT record set. This PR already shows the failure mode: upstream DNS changed, so the test had to be edited. Please move these expectations behind fixtures/MSW, or at least assert only the invariant record(s) witharrayContainingand keep live-DNS checks as separate integration coverage.Also applies to: 38-47, 340-365
🤖 Prompt for AI Agents