georust/geocoding

OpenCage: Deserializing result "components" fails when attempting to parse an integer, when a string is expected

nicklaw5 opened this issue ยท 7 comments

First of all, thanks for the great library!

It appears I'm running into an error when attempting to deserialize the JSON response from a OpenCage forward geocode request.

Here's my code:

fn forward_geocode(place: &str) -> Option<geocoder::Point> {
    let oc = Opencage::new("redacted".to_string());

    let res = oc.forward(&place);
    let res = match res {
        Ok(res) => res,
        Err(e) => panic!("Oops: {:?}", e),
    };

    println!("Got a response: {:?}", res);

    return Some(geocoder::Point{
        lat: res[0].lat(),
        lng: res[0].lng(),
    });
}

and here's the error I'm experiencing:

thread 'tokio-runtime-worker' panicked at 'Oops: Request(reqwest::Error { kind: Decode, source: Error("invalid type: integer `18`, expected a string", line: 1, column: 4362) })', src/server-old.rs:111:19

After inspecting the raw JSON the issue appears to with this line:

pub components: HashMap<String, String>,

where the HashMap expects all values to be of type String. This is not the case given the below response payload that shows the house_number property is in fact an integer:

{
  "documentation": "https://opencagedata.com/api",
  "licenses": [
    {
      "name": "see attribution guide",
      "url": "https://opencagedata.com/credits"
    }
  ],
  "rate": {
    "limit": 2500,
    "remaining": 2490,
    "reset": 1606608000
  },
  "results": [
    {
      "components": {
        "ISO_3166-1_alpha-2": "AU",
        "ISO_3166-1_alpha-3": "AUS",
        "_category": "building",
        "_type": "building",
        "city": "CONCORD",
        "continent": "Oceania",
        "country": "Australia",
        "country_code": "au",
+        "house_number": 18,
        "postcode": "2137",
        "state": "NEW SOUTH WALES",
        "state_code": "NSW",
        "street": "SYDNEY ST"
      },
      "confidence": 10,
      "formatted": "18 SYDNEY ST, CONCORD NSW 2137, Australia",
      "geometry": {
        "lat": -33.8641922,
        "lng": 151.0979607
      }
    }
  ],
  "status": {
    "code": 200,
    "message": "OK"
  },
  "stay_informed": {
    "blog": "https://blog.opencagedata.com",
    "twitter": "https://twitter.com/OpenCage"
  },
  "thanks": "For using an OpenCage API",
  "timestamp": {
    "created_http": "Sat, 28 Nov 2020 20:27:48 GMT",
    "created_unix": 1606595268
  },
  "total_results": 8
}

I'd create a PR to resolve the issue myself but I'm extremely new to Rust. This geocode service I'm writing is my first ever project using the language. I tried making the following change but that didn't seem to do anything. I'm not sure I fully understand how type interfaces work in Rust just yet.

pub struct Results<T>
where
    T: Float,
{
    pub annotations: Option<Annotations<T>>,
    pub bounds: Option<Bounds<T>>,
-    pub components: HashMap<String, String>,
+    pub components: HashMap<String, T>,
    pub confidence: i8,
    pub formatted: String,
    pub geometry: HashMap<String, T>,
}

Thanks for the help!

Hi,

Ed from OpenCage here.

I wonder if we should just make the value of house_number always be a string, as sometimes there are values like "5a" or whatever, that JSON will turn into a string. We do this already with postcode to ensure leading 0s aren't lost.
In hindsight not clear to me why we didn't do this long ago.

Hi,

actually I just dove into the code and definitely the value of house_number should always be a string. I am investigating why that isn't the case here. Hope to have a fix out shortly.

Great! Thanks @freyfogle.

this fix is now live

"components": {
        "ISO_3166-1_alpha-2": "AU",
        "ISO_3166-1_alpha-3": "AUS",
        "_category": "building",
        "_type": "building",
        "city": "CONCORD",
        "continent": "Oceania",
        "country": "Australia",
        "country_code": "au",
        "house_number": "18",
        "postcode": "2137",
        "state": "NEW SOUTH WALES",
        "state_code": "NSW",
        "street": "SYDNEY ST"
      },

thanks again for making me aware, sorry about any inconvenience.

Hi both! Alternatively, I think we can handle this using the string_or_int annotation (if you have a look at line 569 we do something similar there) and convert it on the fly.

well it is fixed now, but just to be safe, yes, you probably should.
Always good practice to assume the upstream datasource can not be trusted.

I can confirm that @freyfogle's patch has resolved my issue. Thanks ๐Ÿ™‚