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

fix dns_1984hosting_add() so checks for HTML responses actually find HTML responses #3541

Merged

Conversation

DerVerruckteFuchs
Copy link
Contributor

In dns_1984hosting.sh, the dns_1984hosting_add() function checks if the response contains an <html> tag. The response I have been getting (when a POST request that adds an _acme-challenge TXT record fails) contains a <!DOCTYPE html> and <html lang="en"> tags but not a plain <html> tag. Therefore, the error response does not get detected. Changing response checking to look for html> instead of <html> properly catches the error response as it covers cases where <!DOCTYPE html>, <html>, and the closing </html> tags are used. One alternative would be to check for all/any <!DOCTYPE html>, <html>, and <html lang="en"> tags. Another alternative would be to check if the response is HTML instead of the expected JSON response. All HTML has a closing </html> tag, so what has been committed should suffice. Since <!DOCTYPE html> is at the top, that should be the first match. Performance wise, this is ideal since _contains does not have to get to the bottom of the page before finding a match for html>.

Earlier explanation of issue from #2851 (comment).

@DerVerruckteFuchs DerVerruckteFuchs changed the title fix dns_1984hosting_add() so checks for HTML responses are actually find HTML responses fix dns_1984hosting_add() so checks for HTML responses actually find HTML responses Jun 6, 2021
@Neilpang Neilpang merged commit afb6c70 into acmesh-official:dev Jun 6, 2021
@DerVerruckteFuchs DerVerruckteFuchs deleted the dns_1984hosting_add()-fix branch April 25, 2022 21:00
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.

None yet

2 participants