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

Dns verification #11

Merged
merged 1 commit into from
Mar 13, 2016
Merged

Dns verification #11

merged 1 commit into from
Mar 13, 2016

Conversation

kfeutz
Copy link
Contributor

@kfeutz kfeutz commented Feb 24, 2016

The DNS-01 challenge requires a TXT record under a validation domain name with “_acme-challenge.” prepended to the domain name being validated (_acme-challenge.example.com). I attempted to use \Amp\Dns\query for DNS record lookup, but was receiving an invalid host name error with this validation domain (because of the leading underscore). Should I open an issue in the \Amp\Dns repo or is my use of dns_get_record($uri) ok here?

This method simply looks up the TXT record under the validation domain and ensures the value attached to the TXT record matches the expected payload for DNS-01 validation.

@kfeutz
Copy link
Contributor Author

kfeutz commented Feb 25, 2016

I had some trouble running my tests with the @ExpectedException tag. I received the following error:

 Failed asserting that exception of type "LogicException" matches expected exception    
 "\Kelunik\Acme\AcmeException". Message was: "Cannot run() recursively; event reactor already active" at ...

I'm pretty sure this has something to do with exceptions being thrown in the \Amp\run(...) block, causing the \Amp\run(...) block to not close the event loop correctly. I did not create an Issue because I am new to PHP and the Amp library, so I am not entirely sure if this is indeed the problem. I created a workaround by closing the event loop (running \Amp\stop()) in a finally {...} block. Does my speculation seem accurate, or am I missing something on my end?

@kelunik
Copy link
Owner

kelunik commented Feb 25, 2016

I will probably have a look at it tomorrow. It should work the way you did it even without Amp\stop(), don't worry, I'll sort this.

I opened an issue in amphp/dns, we will probably resolve it tomorrow as well.

In the mean time, could you squash your commits into a single one? You might also want to correct your git config, so it links your commits properly to your GitHub account.

@bwoebi
Copy link

bwoebi commented Feb 25, 2016

I just fixed that in amp/dns, by removing that arbitrary restriction for query() (as DNS is not bound to URI restrictions).
I'll tag a new version later after a other bug fixes.

@kelunik kelunik merged commit 6699f7c into kelunik:master Mar 13, 2016
@kelunik
Copy link
Owner

kelunik commented Mar 13, 2016

Changed it to use amphp/dns now, thanks!

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

3 participants