Dnspacket comboaddr #4044

Merged
merged 2 commits into from Jul 19, 2016

Projects

None yet

4 participants

@cmouse
Contributor
cmouse commented Jun 26, 2016

No description provided.

@cmouse
Contributor
cmouse commented Jun 26, 2016

Please review.

@Habbie Habbie commented on the diff Jun 26, 2016
pdns/lua-auth.cc
return 1;
}
static int ldp_getRemoteRaw(lua_State *L) {
DNSPacket *p=ldp_checkDNSPacket(L);
- const ComboAddress& ca=p->d_remote;
+ const ComboAddress& ca=p->getRemote();
@Habbie
Habbie Jun 26, 2016 Member

why change this?

@cmouse
cmouse Jun 26, 2016 Contributor

it effectively returns same thing.

@Habbie Habbie and 1 other commented on an outdated diff Jun 26, 2016
pdns/packethandler.cc
@@ -763,8 +763,8 @@ int PacketHandler::trySuperMasterSynchronous(DNSPacket *p, const DNSName& tsigke
try {
Resolver resolver;
uint32_t theirserial;
- resolver.getSoaSerial(p->getRemote(),p->qdomain, &theirserial);
- resolver.resolve(p->getRemote(), p->qdomain, QType::NS, &nsset);
+ resolver.getSoaSerial(p->getRemote().toString(),p->qdomain, &theirserial);
+ resolver.resolve(p->getRemote().toString(), p->qdomain, QType::NS, &nsset);
@Habbie
Habbie Jun 26, 2016 Member

Wonder if these should eventually be taking CA as well :)

@cmouse
cmouse Jun 26, 2016 Contributor

Probably, but not in this change.

@Habbie
Member
Habbie commented Jun 26, 2016

minor nits. LGTM.

@rgacogne rgacogne commented on the diff Jun 26, 2016
pdns/dnspacket.cc
@@ -81,9 +81,9 @@ const string& DNSPacket::getString()
return d_rawpacket;
}
-string DNSPacket::getRemote() const
+ComboAddress DNSPacket::getRemote() const
@rgacogne
rgacogne Jun 26, 2016 Member

const ComboAddress& DNSPacket::getRemote() const, perhaps?

@Habbie
Habbie Jun 26, 2016 Member

Won't & give us risk of leaking if we're not careful? CA is pretty small in any case.

@cmouse
Contributor
cmouse commented Jun 28, 2016

Changed lua backend too.

@cmouse cmouse referenced this pull request Jun 28, 2016
Merged

Update policy lua #4058

@Habbie
Member
Habbie commented Jun 28, 2016

This looks good to me but there is no reason to merge it before 4.0.0, so I suggest doing it right after.

@ahupowerdns ahupowerdns added this to the auth-4.1.0 milestone Jul 1, 2016
@ahupowerdns ahupowerdns merged commit a466a41 into PowerDNS:master Jul 19, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ahupowerdns ahupowerdns modified the milestone: auth-4.1.0 Feb 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment