all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pdm-devel] [RFC PATCH proxmox] pve api types: percent encode string params in url
@ 2025-11-28 12:59 Dominik Csapak
  2025-12-01  8:55 ` Lukas Wagner
  2025-12-01 16:33 ` [pdm-devel] applied: " Thomas Lamprecht
  0 siblings, 2 replies; 3+ messages in thread
From: Dominik Csapak @ 2025-11-28 12:59 UTC (permalink / raw)
  To: pdm-devel

when we generate an url for the pve api from something like

"/some/{foo}/thing/{bar}"

then the url we generate will be wrapped in a `format!` and the
variables `foo` and `bar` will be inserted into the url.

Sometimes, these parameters can contain invalid characters for a url and
we have to percent encode them.

To do that, we convert the url to one with unnamed parameters and add
them with percent encoding.

The order of the url_params is the same as in the url, so we don't
have to to a lookup or similar, simply iterating over that is enough.

This fixes an issue with UPIDs that contain the character '>'.
(e.g. move disk from one vm to another).

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
sending as RFC, because there is a (not so generic) alternative:
we could simply percent encode the upid ourselves in pdm, whenever
we call the pve api.

but since this wasn't too hard to do and everything seems to work
in my (albeit short) tests, I wanted to send it and gather feedback.

 pve-api-types/generator-lib/Schema2Rust.pm | 29 +++++++++++++++++++++-
 pve-api-types/src/client/code.rs           |  2 ++
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/pve-api-types/generator-lib/Schema2Rust.pm b/pve-api-types/generator-lib/Schema2Rust.pm
index ab1c1700..d12c1525 100644
--- a/pve-api-types/generator-lib/Schema2Rust.pm
+++ b/pve-api-types/generator-lib/Schema2Rust.pm
@@ -457,7 +457,27 @@ my sub return_expr : prototype($$) ($def, $expr) {
 my sub format_url : prototype($;$) ($def, $as_ref = 0) {
     if (defined($def->{url_params}) && $def->{url_params}->@*) {
         $as_ref = $as_ref ? '&' : '';
-        return "${as_ref}format!(\"/api2/extjs$def->{url}\")";
+
+        my $url_with_unnamed_params = url_with_unnamed_params($def->{url});
+
+        my $url = "${as_ref}format!(\"/api2/extjs${url_with_unnamed_params}\"";
+
+        # we have to percent encode string parameter in the url
+        for my $url_arg ($def->{url_params}->@*) {
+            my ($arg, $def) = @$url_arg;
+            my $name = $def->{rust_name};
+            my $type = $def->{type};
+
+            if ($type eq '&str') {
+                $url .= ",percent_encode(${name}.as_bytes(), percent_encoding::NON_ALPHANUMERIC)";
+            } elsif ($type =~ /^u8|u16|u32|u64|u128|i8|i16|i32|i64|i128|usize|isize|f32|f64$/) {
+                $url .= ",${name}";
+            } else {
+                $url .= ",percent_encode(${name}.to_string().as_bytes(), percent_encoding::NON_ALPHANUMERIC)";
+            }
+        }
+
+        $url .= ")";
     } else {
         return "\"/api2/extjs$def->{url}\"";
     }
@@ -1617,6 +1637,13 @@ my sub url_parameters : prototype($) {
     return \@params;
 }
 
+# remove named params from the url
+sub url_with_unnamed_params : prototype($) {
+    my ($path) = @_;
+    $path =~ s/\{([^}]+)\}/\{\}/g;
+    return $path;
+}
+
 ### Extract method parameters and deal with path based parameters.
 # $api_method is the dumped schema's method definition.
 my sub method_parameters : prototype($$$$$) {
diff --git a/pve-api-types/src/client/code.rs b/pve-api-types/src/client/code.rs
index ff206ca0..bd6c8ff1 100644
--- a/pve-api-types/src/client/code.rs
+++ b/pve-api-types/src/client/code.rs
@@ -2,6 +2,8 @@
 #![allow(unused)]
 //! The generated API client code.
 
+use percent_encoding::percent_encode;
+
 use proxmox_client::{ApiPathBuilder, ApiResponseData, Error, HttpApiClient};
 
 use crate::types::*;
-- 
2.47.3



_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [pdm-devel] [RFC PATCH proxmox] pve api types: percent encode string params in url
  2025-11-28 12:59 [pdm-devel] [RFC PATCH proxmox] pve api types: percent encode string params in url Dominik Csapak
@ 2025-12-01  8:55 ` Lukas Wagner
  2025-12-01 16:33 ` [pdm-devel] applied: " Thomas Lamprecht
  1 sibling, 0 replies; 3+ messages in thread
From: Lukas Wagner @ 2025-12-01  8:55 UTC (permalink / raw)
  To: Proxmox Datacenter Manager development discussion, Thomas Lamprecht

On Fri Nov 28, 2025 at 1:59 PM CET, Dominik Csapak wrote:
> when we generate an url for the pve api from something like
>
> "/some/{foo}/thing/{bar}"
>
> then the url we generate will be wrapped in a `format!` and the
> variables `foo` and `bar` will be inserted into the url.
>
> Sometimes, these parameters can contain invalid characters for a url and
> we have to percent encode them.
>
> To do that, we convert the url to one with unnamed parameters and add
> them with percent encoding.
>
> The order of the url_params is the same as in the url, so we don't
> have to to a lookup or similar, simply iterating over that is enough.
>
> This fixes an issue with UPIDs that contain the character '>'.
> (e.g. move disk from one vm to another).
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> sending as RFC, because there is a (not so generic) alternative:
> we could simply percent encode the upid ourselves in pdm, whenever
> we call the pve api.

I think handling encoding at this level as implemented in this patch is
exactly right.

>
> but since this wasn't too hard to do and everything seems to work
> in my (albeit short) tests, I wanted to send it and gather feedback.
>
>  pve-api-types/generator-lib/Schema2Rust.pm | 29 +++++++++++++++++++++-
>  pve-api-types/src/client/code.rs           |  2 ++
>  2 files changed, 30 insertions(+), 1 deletion(-)
>

Couldn't find anything that looked off. Everything seems to work fine
after applying this patch, refreshing the bindings and building PDM with
them. Verified in the browser console that non-alphanumeric characters
are indeed percent-encoded. Also spot-checked the diff of the generated code,
everything that I saw looked reasonable.

Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Lukas Wagner <l.wagner@proxmox.com>


_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [pdm-devel] applied: [RFC PATCH proxmox] pve api types: percent encode string params in url
  2025-11-28 12:59 [pdm-devel] [RFC PATCH proxmox] pve api types: percent encode string params in url Dominik Csapak
  2025-12-01  8:55 ` Lukas Wagner
@ 2025-12-01 16:33 ` Thomas Lamprecht
  1 sibling, 0 replies; 3+ messages in thread
From: Thomas Lamprecht @ 2025-12-01 16:33 UTC (permalink / raw)
  To: pdm-devel, Dominik Csapak

On Fri, 28 Nov 2025 13:59:55 +0100, Dominik Csapak wrote:
> when we generate an url for the pve api from something like
> 
> "/some/{foo}/thing/{bar}"
> 
> then the url we generate will be wrapped in a `format!` and the
> variables `foo` and `bar` will be inserted into the url.
> 
> [...]

Applied, thanks!

[1/1] pve api types: percent encode string params in url
      commit: 23310da6face1deeaedcae23de2d36b7dc119226


_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-12-01 16:33 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-28 12:59 [pdm-devel] [RFC PATCH proxmox] pve api types: percent encode string params in url Dominik Csapak
2025-12-01  8:55 ` Lukas Wagner
2025-12-01 16:33 ` [pdm-devel] applied: " Thomas Lamprecht

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal