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

adds findDOHEndpoint() for given zone to JS API #45

Merged
merged 3 commits into from
Jul 31, 2024
Merged

Conversation

adam-burns
Copy link
Collaborator

Adds initial findDOHEndpoint() to WASM JS API

@adam-burns adam-burns added the enhancement New feature or request label Jul 31, 2024
@adam-burns adam-burns self-assigned this Jul 31, 2024
Copy link
Member

@cryptix cryptix left a comment

Choose a reason for hiding this comment

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

SGTM!

a few small nitpicks but nothing that needs to happen now.

golang/wasm/wrapper_js.go Show resolved Hide resolved
}

const div = document.getElementById("domain-doh-endpoint")
if (div.children.length > 0) {
Copy link
Member

Choose a reason for hiding this comment

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

not sure if it is possible to have more then one but instead of removing the first you can also just div.innerHTML = "", i think.

@@ -64,6 +64,7 @@ func FindDOHEndpoint(name string) (*url.URL, error) {
return nil, fmt.Errorf("findDOHEndpoint: no answer section for %s", lookup)
}

// TODO deal with more than one SVCB "_dns." + name in RRSet
Copy link
Member

Choose a reason for hiding this comment

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

good call!

we should start making a list of things we need static test vectors for to drop our need for network i/o during tests and this would definitely be one of them.

@adam-burns adam-burns merged commit b719869 into master Jul 31, 2024
1 check failed
@cryptix cryptix deleted the findDOHEndpoint branch July 31, 2024 15:54
@cryptix cryptix mentioned this pull request Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants