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 DDNSS provider specific IPv6 update flow #270

Closed
wants to merge 1 commit into from
Closed

Fix DDNSS provider specific IPv6 update flow #270

wants to merge 1 commit into from

Conversation

quantum-byte
Copy link

Fixed sending of specific ipv6 information to DDNSS provider.

The DDNSS provider only has one keyword (ip) for specifying the new ip (see documentation.

Currently when an ipv6 ip would be provided the ip6 keyword is filled. This leads to the behaviour that DDNSS picks the ip to fill since the ip keyword is missing. This is not expected behaviour if useProviderIP=false.

I tested it with my own account/setup. Feel free to validate with something like the following:
curl -i "https://www.ddnss.de/upd.php?key=<APIKEY>&host=<HOSTNAME>&ip=<IPV6>"

Fixed the sending of ipv6 information to ddnss provider
@qdm12
Copy link
Owner

qdm12 commented Nov 26, 2021

Thanks!

Although checking https://ddnss.de/info.php and then pressing on "External IP" still shows the ip6 parameter. Can you perhaps confirm with them their documentation is wrong?

@quantum-byte
Copy link
Author

@qdm12 Oh wow i did not see that. I ll check what i can do 😄

@quantum-byte
Copy link
Author

I sent them a description of the issue and request to clarify/fix the documentation via their contact form. I ll give feedback once they respond.

@quantum-byte
Copy link
Author

Just a quick update. I got a response but it was not really helpful.

But it triggered another test on my end and i found out that the ip6 parameter works correctly if you enable the dual-stack setting on your hostname. Then it correctly updates the second ipv6 address. Though if you have it disabled it no longer works.

I sent them another message if the ip6 parameter should work also if the dual-stack stetting is disabled. Lets see what i get back.

@qdm12
Copy link
Owner

qdm12 commented Feb 5, 2022

Hey there any update? Should we just merge it as it is?

@qdm12
Copy link
Owner

qdm12 commented Feb 13, 2022

Hey there any update? Thanks!!

qdm12 added a commit that referenced this pull request Sep 17, 2022
- Discussed on #270
- Original fix proposed by @quantum-byte
@qdm12
Copy link
Owner

qdm12 commented Sep 17, 2022

cd37ab1 should address this problem you have: it adds a boolean setting "dual_stack": false. If it is false, the ip parameter is always used. If it is true and the IP sent is IPv6, then ip6 is used.

I tried force pushing to your fork but I got denied permission unfortunately, so I pushed it myself (mentioning you in the commit) to the master branch.

@qdm12 qdm12 closed this Sep 17, 2022
@qdm12
Copy link
Owner

qdm12 commented Sep 17, 2022

Oh actually the forced push worked, sorry I don't know why my terminal was erroring although it succeeded. Anyway, let's consider this fixed 👍 (even if ddnss.de fixes it on their end)

@quantum-byte
Copy link
Author

Hi @qdm12, sorry for the late reply and thanks for actually implementing a fix.

It works and complies to their specification, even though its not specified anywhere. From the responses i got, they will also not change it ...

@qdm12
Copy link
Owner

qdm12 commented Sep 26, 2022

Awesome thanks for letting me know. Also thanks for contributing, it's always appreciated! 👍

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