public inbox for pdm-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Shannon Sterz" <s.sterz@proxmox.com>
To: "Dominik Csapak" <d.csapak@proxmox.com>
Cc: Proxmox Datacenter Manager development discussion
	<pdm-devel@lists.proxmox.com>
Subject: Re: [pdm-devel] [PATCH datacenter-manager] ui: wizzard/edit remote: validate remote host and port
Date: Fri, 19 Dec 2025 12:40:23 +0100	[thread overview]
Message-ID: <DF25Y3C2QMKZ.2B6CB7ZU7P0HH@proxmox.com> (raw)
In-Reply-To: <610194d7-e3c1-4720-bbd1-d8c9d18529a2@proxmox.com>

On Fri Dec 19, 2025 at 10:05 AM CET, Dominik Csapak wrote:
> s/wizzard/wizard/ in the commit subject
>
> On 12/16/25 4:37 PM, Shannon Sterz wrote:
>> we did not validate these fields before, leading to situations where
>> users could enter a value with a schema here (such as "https://") that
>> lead to errors down the line.
>
> could you elaborate what can lead to errors down the line?
>
> i tried in an unpatched pdm
>
> http://somehost:someport
> and
> https://somehost:someport
>
> and it always behaved like expected?
> e.g. in the wizard we simply strip the http/https part (not intuitive,
> but ok imho)
>
> and the web-url behaves like it should, namely a 'web-url'
>
> and copy/pasting a url from an address bar should work
> (the only thing we might want to do is to strip/disallow the fragment there)

yeah sorry i kind of hurried this, i'll send a proper fix in a minute.
the problem isn't the address/host:port field nor the web url, it's the
table of endpoints. we don't validate the input there and the update
endpoint doesn't even do server side validation. hence, adding a
protocol leads to errors when trying to contact the remote afterward.

>
>
>>
>> Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
>> ---
>> we might not want to call it a "Web UI URL" when editing a remote. a url
>> indicates that i should be able, for example, to copy the url from my
>> browser's address bar and paste it here. this isn't the case. we may
>> want to call it "<IP/Hostname>:Port" just like we do in the add dialog.
>>
>>   lib/pdm-api-types/src/lib.rs          | 3 +++
>>   ui/src/remotes/edit_remote.rs         | 2 ++
>>   ui/src/remotes/wizard_page_connect.rs | 3 ++-
>>   3 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/pdm-api-types/src/lib.rs b/lib/pdm-api-types/src/lib.rs
>> index 5daaa3f..d4cc7ef 100644
>> --- a/lib/pdm-api-types/src/lib.rs
>> +++ b/lib/pdm-api-types/src/lib.rs
>> @@ -137,6 +137,9 @@ pub const HOST_PORT_FORMAT: ApiStringFormat = ApiStringFormat::Pattern(&HOST_POR
>>   pub const HOST_OPTIONAL_PORT_FORMAT: ApiStringFormat =
>>       ApiStringFormat::Pattern(&HOST_OPTIONAL_PORT_REGEX);
>>   pub const HTTP_URL_FORMAT: ApiStringFormat = ApiStringFormat::Pattern(&HTTP_URL_REGEX);
>> +pub const HOST_OPTIONAL_PORT_SCHEMA: Schema = StringSchema::new("A host with an optional port.")
>> +    .format(&HOST_OPTIONAL_PORT_FORMAT)
>> +    .schema();
>>
>>   pub const DAILY_DURATION_FORMAT: ApiStringFormat =
>>       ApiStringFormat::VerifyFn(|s| parse_daily_duration(s).map(drop));
>> diff --git a/ui/src/remotes/edit_remote.rs b/ui/src/remotes/edit_remote.rs
>> index 925d11a..1b6b580 100644
>> --- a/ui/src/remotes/edit_remote.rs
>> +++ b/ui/src/remotes/edit_remote.rs
>> @@ -1,6 +1,7 @@
>>   use std::rc::Rc;
>>
>>   use anyhow::Error;
>> +use pdm_api_types::HOST_OPTIONAL_PORT_SCHEMA;
>>   use serde_json::Value;
>>   use yew::html::IntoEventCallback;
>>   use yew::virtual_dom::{VComp, VNode};
>> @@ -118,6 +119,7 @@ fn edit_remote_input_panel(_form_ctx: &FormContext, remote_id: &str) -> Html {
>>               tr!("Web UI URL"),
>>               Field::new()
>>                   .name("web-url")
>> +                .schema(&HOST_OPTIONAL_PORT_SCHEMA)
>>                   .placeholder(tr!("Use first endpoint.")),
>>           )
>>           .with_custom_child(
>> diff --git a/ui/src/remotes/wizard_page_connect.rs b/ui/src/remotes/wizard_page_connect.rs
>> index fb04f60..b850b5c 100644
>> --- a/ui/src/remotes/wizard_page_connect.rs
>> +++ b/ui/src/remotes/wizard_page_connect.rs
>> @@ -15,7 +15,7 @@ use pwt_macros::builder;
>>   use proxmox_yew_comp::{KVGrid, KVGridRow, SchemaValidation, WizardPageRenderInfo};
>>
>>   use pdm_api_types::remotes::{RemoteType, TlsProbeOutcome};
>> -use pdm_api_types::CERT_FINGERPRINT_SHA256_SCHEMA;
>> +use pdm_api_types::{CERT_FINGERPRINT_SHA256_SCHEMA, HOST_OPTIONAL_PORT_SCHEMA};
>>   use proxmox_acme_api::CertificateInfo;
>>
>>   #[derive(Clone, PartialEq, Properties)]
>> @@ -242,6 +242,7 @@ impl Component for PdmWizardPageConnect {
>>                   Field::new()
>>                       .name("hostname")
>>                       .placeholder(tr!("<IP/Hostname>:Port"))
>> +                    .schema(&HOST_OPTIONAL_PORT_SCHEMA)
>>                       .required(true),
>>               )
>>               .with_large_field(
>> --
>> 2.47.3
>>
>>
>>
>> _______________________________________________
>> pdm-devel mailing list
>> pdm-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
>>
>>



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


  reply	other threads:[~2025-12-19 11:39 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-16 15:37 Shannon Sterz
2025-12-19  9:05 ` Dominik Csapak
2025-12-19 11:40   ` Shannon Sterz [this message]
2025-12-19 12:50 ` Shannon Sterz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DF25Y3C2QMKZ.2B6CB7ZU7P0HH@proxmox.com \
    --to=s.sterz@proxmox.com \
    --cc=d.csapak@proxmox.com \
    --cc=pdm-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal