From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 09331C1EA for ; Thu, 14 Sep 2023 12:41:32 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id DE5BD376C4 for ; Thu, 14 Sep 2023 12:41:31 +0200 (CEST) Received: from bookworm-dev.localdomain (unknown [94.136.29.99]) by firstgate.proxmox.com (Proxmox) with ESMTP for ; Thu, 14 Sep 2023 12:41:30 +0200 (CEST) Received: by bookworm-dev.localdomain (Postfix, from userid 1000) id ADD3360575; Thu, 14 Sep 2023 12:41:30 +0200 (CEST) From: Dominik Csapak To: pbs-devel@lists.proxmox.com Date: Thu, 14 Sep 2023 12:41:30 +0200 Message-Id: <20230914104130.59082-1-d.csapak@proxmox.com> X-Mailer: git-send-email 2.39.2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.385 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RDNS_NONE 0.793 Delivered to internal network by a host with no rDNS SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pbs-devel] [PATCH proxmox] client: fix optional data for errors X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 14 Sep 2023 10:41:32 -0000 previously we changed the internal type of the 'data' property from Option to T in the assumption the api always returns 'data:null'. this is actually only the case when the api call succeeds. in an error case there is no data property at all. to fix this issue while behaving the same for 'data:null' we have to revert to Option for RawApiResponse but instead of always throwing an error for 'data:null' in 'check' we now try there to deserialize from Value::Null for T if there was no data. This will succeed for the Type '()' which was the motivation for the original change. The only downside is that the RawApiResponse now has a trait bound that T is deserializeable, but was a requirement for using it anyway (as there was no other way of constructing it) Fixes: 271a55f ("client: remove option from inner RawApiResponse") Signed-off-by: Dominik Csapak --- proxmox-client/src/lib.rs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/proxmox-client/src/lib.rs b/proxmox-client/src/lib.rs index 9aa7144..8161cc0 100644 --- a/proxmox-client/src/lib.rs +++ b/proxmox-client/src/lib.rs @@ -106,7 +106,7 @@ impl HttpApiResponse { /// Expect that the API call did *not* return any data in the `data` field. pub fn nodata(self) -> Result<(), Error> { - let response = serde_json::from_slice::>>(&self.body) + let response = serde_json::from_slice::>(&self.body) .map_err(|err| Error::bad_api("unexpected api response", err))?; if response.data.is_some() { @@ -131,7 +131,7 @@ struct RawApiResponse { message: Option, #[serde(default, deserialize_with = "proxmox_login::parse::deserialize_bool")] success: Option, - data: T, + data: Option, #[serde(default)] errors: HashMap, @@ -140,7 +140,10 @@ struct RawApiResponse { attribs: HashMap, } -impl RawApiResponse { +impl RawApiResponse +where + T: for<'de> Deserialize<'de>, +{ fn check_success(mut self) -> Result { if self.success == Some(true) { return Ok(self); @@ -163,8 +166,16 @@ impl RawApiResponse { fn check(self) -> Result, Error> { let this = self.check_success()?; + // RawApiResponse has no data, but this also happens for Value::Null, and T + // might be deserializeable from that, so try here again + let data = match this.data { + Some(data) => data, + None => serde_json::from_value(Value::Null) + .map_err(|_| Error::BadApi("api returned no data".to_string(), None))?, + }; + Ok(ApiResponseData { - data: this.data, + data, attribs: this.attribs, }) } -- 2.39.2