From: Pieter Lexis Date: Fri, 8 Dec 2017 14:33:11 +0000 (+0100) Subject: API: return 404 for non-existing zones X-Git-Tag: dnsdist-1.3.0~195^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F6076%2Fhead;p=thirdparty%2Fpdns.git API: return 404 for non-existing zones This commit ensures that any request for a path inside a non-existing zone returns an HTTP 404 code. Closes #6074 --- diff --git a/pdns/ws-auth.cc b/pdns/ws-auth.cc index 70f4ce779d..fe15dcc03b 100644 --- a/pdns/ws-auth.cc +++ b/pdns/ws-auth.cc @@ -329,8 +329,9 @@ static bool shouldDoRRSets(HttpRequest* req) { static void fillZone(const DNSName& zonename, HttpResponse* resp, bool doRRSets) { UeberBackend B; DomainInfo di; - if(!B.getDomainInfo(zonename, di)) - throw ApiException("Could not find domain '"+zonename.toString()+"'"); + if(!B.getDomainInfo(zonename, di)) { + throw HttpNotFoundException(); + } DNSSECKeeper dk(&B); Json::object doc = getZoneInfo(di, &dk); @@ -723,8 +724,9 @@ static void apiZoneMetadata(HttpRequest* req, HttpResponse *resp) { UeberBackend B; DomainInfo di; - if (!B.getDomainInfo(zonename, di)) - throw ApiException("Could not find domain '"+zonename.toString()+"'"); + if (!B.getDomainInfo(zonename, di)) { + throw HttpNotFoundException(); + } if (req->method == "GET") { map > md; @@ -807,8 +809,9 @@ static void apiZoneMetadataKind(HttpRequest* req, HttpResponse* resp) { UeberBackend B; DomainInfo di; - if (!B.getDomainInfo(zonename, di)) - throw ApiException("Could not find domain '"+zonename.toString()+"'"); + if (!B.getDomainInfo(zonename, di)) { + throw HttpNotFoundException(); + } string kind = req->parameters["kind"]; @@ -1108,8 +1111,9 @@ static void apiZoneCryptokeys(HttpRequest *req, HttpResponse *resp) { UeberBackend B; DNSSECKeeper dk(&B); DomainInfo di; - if (!B.getDomainInfo(zonename, di)) - throw HttpBadRequestException(); + if (!B.getDomainInfo(zonename, di)) { + throw HttpNotFoundException(); + } int inquireKeyId = -1; if (req->parameters.count("key_id")) { @@ -1366,12 +1370,14 @@ static void apiServerZones(HttpRequest* req, HttpResponse* resp) { static void apiServerZoneDetail(HttpRequest* req, HttpResponse* resp) { DNSName zonename = apiZoneIdToName(req->parameters["id"]); + UeberBackend B; + DomainInfo di; + if (!B.getDomainInfo(zonename, di)) { + throw HttpNotFoundException(); + } + if(req->method == "PUT" && !::arg().mustDo("api-readonly")) { // update domain settings - UeberBackend B; - DomainInfo di; - if(!B.getDomainInfo(zonename, di)) - throw ApiException("Could not find domain '"+zonename.toString()+"'"); updateDomainSettingsFromDocument(B, di, zonename, req->json()); @@ -1381,11 +1387,6 @@ static void apiServerZoneDetail(HttpRequest* req, HttpResponse* resp) { } else if(req->method == "DELETE" && !::arg().mustDo("api-readonly")) { // delete domain - UeberBackend B; - DomainInfo di; - if(!B.getDomainInfo(zonename, di)) - throw ApiException("Could not find domain '"+zonename.toString()+"'"); - if(!di.backend->deleteDomain(zonename)) throw ApiException("Deleting domain '"+zonename.toString()+"' failed: backend delete failed/unsupported"); @@ -1400,7 +1401,6 @@ static void apiServerZoneDetail(HttpRequest* req, HttpResponse* resp) { fillZone(zonename, resp, shouldDoRRSets(req)); return; } - throw HttpMethodNotAllowedException(); } @@ -1414,8 +1414,9 @@ static void apiServerZoneExport(HttpRequest* req, HttpResponse* resp) { UeberBackend B; DomainInfo di; - if(!B.getDomainInfo(zonename, di)) - throw ApiException("Could not find domain '"+zonename.toString()+"'"); + if (!B.getDomainInfo(zonename, di)) { + throw HttpNotFoundException(); + } DNSResourceRecord rr; SOAData sd; @@ -1448,8 +1449,9 @@ static void apiServerZoneAxfrRetrieve(HttpRequest* req, HttpResponse* resp) { UeberBackend B; DomainInfo di; - if(!B.getDomainInfo(zonename, di)) - throw ApiException("Could not find domain '"+zonename.toString()+"'"); + if (!B.getDomainInfo(zonename, di)) { + throw HttpNotFoundException(); + } if(di.masters.empty()) throw ApiException("Domain '"+zonename.toString()+"' is not a slave domain (or has no master defined)"); @@ -1467,8 +1469,9 @@ static void apiServerZoneNotify(HttpRequest* req, HttpResponse* resp) { UeberBackend B; DomainInfo di; - if(!B.getDomainInfo(zonename, di)) - throw ApiException("Could not find domain '"+zonename.toString()+"'"); + if (!B.getDomainInfo(zonename, di)) { + throw HttpNotFoundException(); + } if(!Communicator.notifyDomain(zonename)) throw ApiException("Failed to add to the queue - see server log"); @@ -1484,8 +1487,9 @@ static void apiServerZoneRectify(HttpRequest* req, HttpResponse* resp) { UeberBackend B; DomainInfo di; - if(!B.getDomainInfo(zonename, di)) - throw ApiException("Could not find domain '"+zonename.toString()+"'"); + if (!B.getDomainInfo(zonename, di)) { + throw HttpNotFoundException(); + } DNSSECKeeper dk(&B); @@ -1584,8 +1588,9 @@ static void patchZone(HttpRequest* req, HttpResponse* resp) { UeberBackend B; DomainInfo di; DNSName zonename = apiZoneIdToName(req->parameters["id"]); - if (!B.getDomainInfo(zonename, di)) - throw ApiException("Could not find domain '"+zonename.toString()+"'"); + if (!B.getDomainInfo(zonename, di)) { + throw HttpNotFoundException(); + } vector new_records; vector new_comments; diff --git a/regression-tests.api/test_Zones.py b/regression-tests.api/test_Zones.py index 02896cd0c8..a3d8bd19d5 100644 --- a/regression-tests.api/test_Zones.py +++ b/regression-tests.api/test_Zones.py @@ -500,8 +500,9 @@ class AuthZones(ApiTestCase, AuthZonesHelperMixin): payload_metadata = {"type": "Metadata", "kind": "AXFR-SOURCE", "metadata": ["127.0.0.2"]} r = self.session.post(self.url("/api/v1/servers/localhost/zones/idonotexist.123.456.example./metadata"), data=json.dumps(payload_metadata)) - self.assertEquals(r.status_code, 422) - self.assertIn('Could not find domain ', r.json()['error']) + self.assertEquals(r.status_code, 404) + # Note: errors should probably contain json (see #5988) + # self.assertIn('Could not find domain ', r.json()['error']) def test_create_slave_zone(self): # Test that nameservers can be absent for slave zones. diff --git a/regression-tests.api/test_cryptokeys.py b/regression-tests.api/test_cryptokeys.py index cccb3c783f..553f2198d0 100644 --- a/regression-tests.api/test_cryptokeys.py +++ b/regression-tests.api/test_cryptokeys.py @@ -47,11 +47,16 @@ class Cryptokeys(ApiTestCase): except subprocess.CalledProcessError as e: self.fail("pdnsutil list-keys failed: " + e.output) + def test_get_wrong_zone(self): + self.keyid = self.add_zone_key() + r = self.session.get(self.url("/api/v1/servers/localhost/zones/"+self.zone+"fail/cryptokeys/"+self.keyid)) + self.assertEquals(r.status_code, 404) + def test_delete_wrong_zone(self): self.keyid = self.add_zone_key() #checks for not covered zonename r = self.session.delete(self.url("/api/v1/servers/localhost/zones/"+self.zone+"fail/cryptokeys/"+self.keyid)) - self.assertEquals(r.status_code, 400) + self.assertEquals(r.status_code, 404) def test_delete_key_is_gone(self): self.keyid = self.add_zone_key()