From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michael Tremer <michael.tremer@ipfire.org>
To: location@lists.ipfire.org
Subject: Re: [PATCH] location-importer: Fix Spamhaus ASN-DROP parsing
Date: Sun, 05 Nov 2023 13:47:53 +0000
Message-ID: <2ABB0AF4-CC9C-4CEA-A389-2452CC3DE685@ipfire.org>
In-Reply-To: <fd183979-30db-463a-a381-4894cf4c8033@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============7934356550214959854=="
List-Id: <location.lists.ipfire.org>

--===============7934356550214959854==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

Hello Peter,

> On 4 Nov 2023, at 14:04, Peter M=C3=BCller <peter.mueller(a)ipfire.org> wro=
te:
>=20
> The format of this list has changed, from a plain text file with a
> customer schema to JSON. Adjust our routines accordingly to make use of
> this list again.
>=20
> Signed-off-by: Peter M=C3=BCller <peter.mueller(a)ipfire.org>
> Tested-by: Peter M=C3=BCller <peter.mueller(a)ipfire.org>
> ---
> src/scripts/location-importer.in | 21 +++++++--------------
> 1 file changed, 7 insertions(+), 14 deletions(-)
>=20
> diff --git a/src/scripts/location-importer.in b/src/scripts/location-import=
er.in
> index 28a4f6c..8b0c676 100644
> --- a/src/scripts/location-importer.in
> +++ b/src/scripts/location-importer.in
> @@ -3,7 +3,7 @@
> #                                                                          =
   #
> # libloc - A library to determine the location of someone on the Internet  =
   #
> #                                                                          =
   #
> -# Copyright (C) 2020-2022 IPFire Development Team <info(a)ipfire.org>     =
      #
> +# Copyright (C) 2020-2023 IPFire Development Team <info(a)ipfire.org>     =
      #
> #                                                                          =
   #
> # This library is free software; you can redistribute it and/or            =
   #
> # modify it under the terms of the GNU Lesser General Public               =
   #
> @@ -1686,7 +1686,7 @@ class CLI(object):
> ]
>=20
> asn_lists =3D [
> - ("SPAMHAUS-ASNDROP", "https://www.spamhaus.org/drop/asndrop.txt")
> + ("SPAMHAUS-ASNDROP", "https://www.spamhaus.org/drop/asndrop.json")
> ]
>=20
> for name, url in ip_lists:
> @@ -1759,23 +1759,16 @@ class CLI(object):
>=20
> # Iterate through every line, filter comments and add remaining ASNs to
> # the override table in case they are valid...
> - for sline in f.readlines():
> + for sline in fcontent:
> # The response is assumed to be encoded in UTF-8...
> sline =3D sline.decode("utf-8")
>=20
> - # Comments start with a semicolon...
> - if sline.startswith(";"):
> + # Load every line as a JSON object and try to obtain an ASN from it...
> + try:
> + asn =3D json.loads(sline)["asn"]
> + except KeyError:
> continue

It would be nicer if you didn=E2=80=99t do so many things in one line, becaus=
e it will get difficult to catch the correct exception.

I believe that this should be split into the json.loads() operation where you=
 should catch any JSON decoding issues. This would make it clear that some ki=
nd of download issue or similar has happened.

Fetching the =E2=80=9Casn=E2=80=9D field should be a second step, because an =
error here means that you have received a valid JSON object, but the format h=
as changed.

> - # Throw away anything after the first space...
> - sline =3D sline.split()[0]
> -
> - # ... strip the "AS" prefix from it ...
> - sline =3D sline.strip("AS")
> -
> - # ... and convert it into an integer. Voila.
> - asn =3D int(sline)
> -
> # Filter invalid ASNs...
> if not self._check_parsed_asn(asn):
> log.warning("Skipping bogus ASN found in %s (%s): %s" % \
> --=20
> 2.35.3


--===============7934356550214959854==--