feat: dnslink support #408
Conversation
luissanchez0305
requested review from
vojtechsimetka and
AuHau
as code owners
There was a problem hiding this comment.
Soooo again, I started to do a thorough review, but then I ran into one conceptional misunderstanding. How I understand it from @agazso is that, basically you should not have to specify the domainLookup setting as you have now, but instead when the requests come to the Proxy, take the Request's Hostname and perform DNSLink discover on it. So that would mean replacing the domain on this line with req.hostname instead. There is still problem with the requestFilter because with this approach there will be some performance hit (as upon every requests there needs to be DNSLink check).
Lets discuss it on today's call.
There was a problem hiding this comment.
Have you tested it on root domains? I am not sure how this work with having employed the requestFilter where there is a check for having subdomains?
Bzz.link and DNS.link functionality are not really related. Could you please split it into separate files? I would even suggest having two createProxyMiddleware instances one for Bzz.link and one for DNSLink. With that you could implement custom requestFilter for the DNSLink that would perform the validation of the domains... Also it would simplify a lot the server.ts file.
| @@ -23,20 +27,100 @@ export function requestFilter(pathname: string, req: Request): boolean { | |||
| return req.subdomains.length >= 1 | |||
| } | |||
|
|
|||
| export function getDomain(req: Request): string | undefined { | |||
| try { | |||
| const matches = req.headers.host!.match(/^([^?#]*)(\?([^#]*))?(#(.*))?:[0-9]*]?/i) // clean host domain string | |||
There was a problem hiding this comment.
Why do you do it like this? IMHO you should use req.hostname as I have already mentioned previously several times.
| router: bzzLink.routerClosure(beeApiUrl, Boolean(cidSubdomains), Boolean(ensSubdomains)), | ||
| router: bzzLink.routerClosure( | ||
| beeApiUrl, | ||
| domainsLookup!, |
There was a problem hiding this comment.
The ! does not make sense, it should allow undefined instead no?
| let domain = getDomain(req) | ||
|
|
||
| // in case domain is not on the list of allowed domains then it uses anything on headers-host | ||
| if (!(domain && validateDomain(domain, dnslinkDomains))) { |
There was a problem hiding this comment.
This does not make sense to me. If the domain is not listed in valid domains then no DNSLink should be looked up, no?
| function getDnslinkBzzRoute(hostname: string, target: string, result: Result | undefined): string | undefined { | ||
| if (result) { | ||
| const { txtEntries } = result | ||
| const txtEntry = JSON.parse(JSON.stringify(txtEntries[0])) |
Closes #98
Currently Bee (and the gateway-proxy) only supports ENS domains. The issue with those are that it's quite expensive to buy them but more importantly it's also quite expensive to update or modify them because of mainnet gas prices.
One idea is to support dnslink on the gateway-proxy, at least to some degree. For example if the proxy served unknown A records by looking up their dnslink TXT record then it classic DNS domains could still work but the content could be served through the proxy.