[pve-devel] [PATCH http-server 1/3] formatter: html: remove unused code

[pve-devel] [PATCH http-server 1/3] formatter: html: remove unused code

[Dominik Csapak]

the console js code was added in
 63307be (add generic formatter framework
in January 2017 and marked to be removed (in the FIXME) with
 6189d2e (Formatter/Bootstrap.pm; use configured cookie_name)
a day later.

This was never used, so simply remove it.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/PVE/APIServer/Formatter/Bootstrap.pm | 51 ------------------------
 1 file changed, 51 deletions(-)

diff --git a/src/PVE/APIServer/Formatter/Bootstrap.pm b/src/PVE/APIServer/Formatter/Bootstrap.pm
index 6be0049..be37441 100644
--- a/src/PVE/APIServer/Formatter/Bootstrap.pm
+++ b/src/PVE/APIServer/Formatter/Bootstrap.pm
@@ -7,58 +7,8 @@ use HTML::Entities;
 use JSON;
 use URI::Escape;
 
-# FIXME: remove console code??
-
 # Helpers to generate simple html pages using Bootstrap markup.
 
-my $jssrc = <<_EOJS;
-PVE.open_vm_console = function(node, vmid) {
-    console.log("open vm " + vmid + " on node " + node);
-
-    var downloadWithName = function(uri, name) {
-	var link =  jQuery('#pve_console_anchor');
-	link.attr("href", uri);
-
-	// Note: we need to tell android the correct file name extension
-	// but we do not set 'download' tag for other environments, because
-	// It can have strange side effects (additional user prompt on firefox)
-	var andriod = navigator.userAgent.match(/Android/i) ? true : false;
-	if (andriod) {
-	    link.attr("download", name);
-	}
-
-	if (document.createEvent) {
-	    var evt = document.createEvent("MouseEvents");
-	    evt.initMouseEvent('click', true, true, window, 1, 0, 0, 0, 0, false, false, false, false, 0, null);
-	    link.get(0).dispatchEvent(evt);
-	} else {
-	    link.get(0).fireEvent('onclick');
-	}
-    };
-
-    jQuery.ajax("/api2/json/console", {
-      data: { vmid: vmid, node: node },
-      headers: { CSRFPreventionToken: PVE.CSRFPreventionToken },
-      dataType: 'json',
-      type: 'POST',
-      error: function(jqXHR, textStatus, errorThrown) {
-	  // fixme: howto view JS errors ?
-	  console.log("ERROR " +  textStatus + ": " + errorThrown);
-      },
-      success:   function(data) {
-	  var raw = "[virt-viewer]\\n";
-	  jQuery.each(data.data, function(k, v) {
-	      raw += k + "=" + v + "\\n";
-	  });
-	  var url = 'data:application/x-virt-viewer;charset=UTF-8,' +
-	      encodeURIComponent(raw);
-
-	  downloadWithName(url, "pve-spice.vv");
-      }
-    });
-};
-_EOJS
-
 sub new {
     my ($class, $res, $url, $auth, $config) = @_;
 
@@ -107,7 +57,6 @@ sub body {
 
 <script type="text/javascript">
 $jssetup
-$jssrc
 </script>
 
     <style>
-- 
2.39.5



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

[pve-devel] [PATCH http-server 2/3] formatter: html: fix logout button

[Dominik Csapak]

in commit
 d0f4b94 (fix regression in api/html (bootstrap) viewer)

the $unsafe parameter of uri_escape_utf8 was corrected. This
unintentionally also escapes the 'onclick' content of the logout button,
making it not valid javascript code and thus would not execute.

The commit talks about it being broken since URI::Escape v5.13, but it
was seemingly broken before that too (tested on a PVE 7.x install with
URI::Escape version 5.08) in that it did not escape anything on PVE 7.

To fix the unintentional escape here, add 'onclick' to the exemptions of
the escaped attributes. This should be safe since we don't add any user
supplied value into these.

While at it, rename 'onClick' to 'onclick' to be consistent with the
other attribute names we use.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/PVE/APIServer/Formatter/Bootstrap.pm | 1 +
 src/PVE/APIServer/Formatter/HTML.pm      | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/PVE/APIServer/Formatter/Bootstrap.pm b/src/PVE/APIServer/Formatter/Bootstrap.pm
index be37441..0055d64 100644
--- a/src/PVE/APIServer/Formatter/Bootstrap.pm
+++ b/src/PVE/APIServer/Formatter/Bootstrap.pm
@@ -113,6 +113,7 @@ sub el {
 
     my $noescape = {
 	placeholder => 1,
+	onclick => 1,
     };
 
     foreach my $attr (keys %param)  {
diff --git a/src/PVE/APIServer/Formatter/HTML.pm b/src/PVE/APIServer/Formatter/HTML.pm
index 80617ca..2ce0723 100644
--- a/src/PVE/APIServer/Formatter/HTML.pm
+++ b/src/PVE/APIServer/Formatter/HTML.pm
@@ -34,7 +34,7 @@ sub render_page {
 	cn => {
 	    tag => 'a',
 	    href => $get_portal_login_url->($config),
-	    onClick => "PVE.delete_auth_cookie();",
+	    onclick => "PVE.delete_auth_cookie();",
 	    text => "Logout",
 	}};
 
-- 
2.39.5



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

[pve-devel] [PATCH http-server 3/3] formatter: html: update to bootstrap 5

[Dominik Csapak]

this makes a few changes necessary, but not too much:
* include the different directory for bootstrap5
* use different navbar markup
* different classes for navbar container + items
* add classes to pre tag since it's not styled anymore in newer
  bootstrap versions
* add 'form-label' to labels
* use containers with 'mb-3' for form + buttons
* use 'd-grid' container for button instead of 'btn-block'
* add 'breadcrumb-item' where necessary

Since bootstrap 5 does not depend on jQuery anymore, use that chance to
remove it here as dependency too. For that remove the 'button'
and 'add_js' subs that were never actually used.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 debian/control                           |   3 +-
 src/PVE/APIServer/AnyEvent.pm            |   3 +
 src/PVE/APIServer/Formatter/Bootstrap.pm |  37 +------
 src/PVE/APIServer/Formatter/HTML.pm      | 132 +++++++++++++----------
 4 files changed, 84 insertions(+), 91 deletions(-)

diff --git a/debian/control b/debian/control
index 0d0161e..4ce0368 100644
--- a/debian/control
+++ b/debian/control
@@ -15,8 +15,7 @@ Depends: libanyevent-http-perl,
          libhttp-date-perl,
          libhttp-message-perl,
          libio-socket-ssl-perl,
-         libjs-bootstrap,
-         libjs-jquery,
+         libjs-bootstrap5,
          libjson-perl,
          libnet-ip-perl,
          libpve-common-perl (>= 8.0.2),
diff --git a/src/PVE/APIServer/AnyEvent.pm b/src/PVE/APIServer/AnyEvent.pm
index b71a9a5..9aeae2f 100644
--- a/src/PVE/APIServer/AnyEvent.pm
+++ b/src/PVE/APIServer/AnyEvent.pm
@@ -2032,6 +2032,9 @@ sub new {
     my $glyphicons = '/usr/share/fonts/truetype/glyphicons/';
     add_dirs($self->{dirs}, '/js/bootstrap/fonts/' => "$glyphicons");
 
+    # libjs-bootstrap5 uses a different dir with symlinks
+    add_dirs($self->{dirs}, '/js/bootstrap5/' => "/usr/share/bootstrap-html");
+
     # init inotify
     PVE::INotify::inotify_init();
 
diff --git a/src/PVE/APIServer/Formatter/Bootstrap.pm b/src/PVE/APIServer/Formatter/Bootstrap.pm
index 0055d64..911caac 100644
--- a/src/PVE/APIServer/Formatter/Bootstrap.pm
+++ b/src/PVE/APIServer/Formatter/Bootstrap.pm
@@ -53,7 +53,7 @@ sub body {
     <title>$self->{title}</title>
 
     <!-- Bootstrap -->
-    <link href="/js/bootstrap/css/bootstrap.min.css" rel="stylesheet">
+    <link href="/js/bootstrap5/css/bootstrap.min.css" rel="stylesheet">
 
 <script type="text/javascript">
 $jssetup
@@ -65,10 +65,8 @@ body {
 }
     </style>
 
-    <!-- jQuery (necessary for Bootstrap's JavaScript plugins) -->
-    <script src="/js/jquery/jquery.min.js"></script>
-    <!-- Include all compiled plugins (below), or include individual files as needed -->
-    <script src="/js/bootstrap/js/bootstrap.min.js"></script>
+    <!-- Include bootstrap bundle (everything necessary to run) -->
+    <script src="/js/bootstrap5/js/bootstrap.bundle.min.js"></script>
 
   </head>
   <body>
@@ -155,33 +153,4 @@ sub alert {
     return $self->el(class => "alert alert-danger", %param);
 }
 
-sub add_js {
-    my ($self, $js) = @_;
-
-    $self->{js} .= $js . "\n";
-}
-
-my $format_event_callback = sub {
-    my ($info) = @_;
-
-    my $pstr = encode_json($info->{param});
-    return "function(e){$info->{fn}.apply(e, $pstr);}";
-};
-
-sub button {
-    my ($self, %param) = @_;
-
-    $param{tag} = 'button';
-    $param{class} = "btn btn-default btn-xs";
-
-    if (my $click = delete $param{click}) {
-	my ($html, $id) = $self->el(%param);
-	my $cb = &$format_event_callback($click);
-	$self->add_js("jQuery('#$id').on('click', $cb);");
-	return $html;
-    } else {
-	return $self->el(%param);
-    }
-}
-
 1;
diff --git a/src/PVE/APIServer/Formatter/HTML.pm b/src/PVE/APIServer/Formatter/HTML.pm
index 2ce0723..afda118 100644
--- a/src/PVE/APIServer/Formatter/HTML.pm
+++ b/src/PVE/APIServer/Formatter/HTML.pm
@@ -27,75 +27,86 @@ my $get_portal_login_url = sub {
 sub render_page {
     my ($doc, $html, $config) = @_;
 
-    my $items = [];
-
-    push @$items, {
-	tag => 'li',
-	cn => {
-	    tag => 'a',
-	    href => $get_portal_login_url->($config),
-	    onclick => "PVE.delete_auth_cookie();",
-	    text => "Logout",
-	}};
-
     my $base_url = $get_portal_base_url->($config);
 
     my $nav = $doc->el(
-	class => "navbar navbar-inverse navbar-fixed-top",
-	role => "navigation", cn => {
-	    class => "container", cn => [
+	class => "navbar navbar-dark navbar-expand-lg bg-dark fixed-top",
+	'data-bs-theme' => 'dark',
+	role => "navigation",
+	cn => {
+	    class => "container",
+	    cn => [
 		{
-		    class => "navbar-header", cn => [
-			{
-			    tag => 'button',
-			    type => 'button',
-			    class => "navbar-toggle",
-			    'data-toggle' => "collapse",
-			    'data-target' => ".navbar-collapse",
-			    cn => [
-				{ tag => 'span', class => 'sr-only', text => "Toggle navigation" },
-				{ tag => 'span', class => 'icon-bar' },
-				{ tag => 'span', class => 'icon-bar' },
-				{ tag => 'span', class => 'icon-bar' },
-			    ],
-			},
+		    tag => 'a',
+		    class => "navbar-brand",
+		    href => $base_url,
+		    text => $config->{title},
+		},
+		{
+		    tag => 'button',
+		    type => 'button',
+		    class => "navbar-toggler",
+		    'data-bs-toggle' => "collapse",
+		    'data-bs-target' => ".navbarNav",
+		    cn => [
 			{
-			    tag => 'a',
-			    class => "navbar-brand",
-			    href => $base_url,
-			    text => $config->{title},
+			    tag => 'span',
+			    class => 'navbar-toggler-icon',
 			},
 		    ],
 		},
 		{
-		    class => "collapse navbar-collapse",
+		    class => "collapse navbar-collapse navbarNav",
 		    cn => {
 			tag => 'ul',
-			class => "nav navbar-nav",
-			cn => $items,
+			class => "navbar-nav",
+			cn => [
+			    {
+				tag => 'li',
+				class => 'nav-item',
+				cn => {
+				    tag => 'a',
+				    class => 'nav-link',
+				    href => $get_portal_login_url->($config),
+				    onclick => "PVE.delete_auth_cookie",
+				    text => "Logout",
+				},
+			    }
+			],
 		    },
 		},
-	    ],
-	});
+	    ]
+	}
+    );
 
-    $items = [];
+    my $items = [];
     my @pcomp = split('/', $doc->{url});
     shift @pcomp; # empty
     shift @pcomp; # api2
     shift @pcomp; # $format
 
     my $href = $base_url;
-    push @$items, { tag => 'li', cn => {
-	tag => 'a',
-	href => $href,
-	text => 'Home'}};
-
-    foreach my $comp (@pcomp) {
-	$href .= "/".encode_entities($comp);
-	push @$items, { tag => 'li', cn => {
+    push @$items, {
+	tag => 'li',
+	class => 'breadcrumb-item',
+	cn => {
 	    tag => 'a',
 	    href => $href,
-	    text => $comp}};
+	    text => 'Home',
+	},
+    };
+
+    foreach my $comp (@pcomp) {
+	$href .= "/" . encode_entities($comp);
+	push @$items, {
+	    tag => 'li',
+	    class => 'breadcrumb-item',
+	    cn => {
+		tag => 'a',
+		href => $href,
+		text => $comp,
+	    },
+	};
     }
 
     my $breadcrumbs = $doc->el(tag => 'ol', class => 'breadcrumb container', cn => $items);
@@ -114,6 +125,7 @@ my $login_form = sub {
     my $items = [
 	{
 	    tag => 'label',
+	    class => 'form-label',
 	    text => "Please sign in",
 	},
 	{
@@ -150,14 +162,24 @@ my $login_form = sub {
 	    action => $get_portal_login_url->($config),
 	    cn => [
 		{
-		    class => 'form-group',
-		    cn => $items,
+		    class => "mb-3",
+		    cn => [
+			{
+			    class => 'form-group',
+			    cn => $items,
+			},
+		    ],
 		},
 		{
-		    tag => 'button',
-		    type => 'submit',
-		    class => 'btn btn-lg btn-primary btn-block',
-		    text => "Sign in",
+		    class => "d-grid",
+		    cn => [
+			{
+			    tag => 'button',
+			    type => 'submit',
+			    class => 'btn btn-lg btn-primary',
+			    text => "Sign in",
+			},
+		    ],
 		},
 	    ],
 	});
@@ -236,13 +258,13 @@ PVE::APIServer::Formatter::register_formatter($portal_format, sub {
 	} else {
 
 	    my $json = to_json($data, {allow_nonref => 1, pretty => 1, canonical => 1});
-	    $html .= $doc->el(tag => 'pre', text => $json);
+	    $html .= $doc->el(tag => 'pre', class => 'bg-light border rounded p-2', text => $json);
  	}
 
     } else {
 
 	my $json = to_json($data, {allow_nonref => 1, pretty => 1, canonical => 1});
-	$html .= $doc->el(tag => 'pre', text => $json);
+        $html .= $doc->el(tag => 'pre', class => 'bg-light border rounded p-2', text => $json);
     }
 
     $html = $doc->el(class => 'container', html => $html);
-- 
2.39.5



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

Re: [pve-devel] [PATCH http-server 3/3] formatter: html: update to bootstrap 5

[Thomas Lamprecht]

Am 03.06.25 um 15:04 schrieb Dominik Csapak:
> this makes a few changes necessary, but not too much:
> * include the different directory for bootstrap5
> * use different navbar markup
> * different classes for navbar container + items
> * add classes to pre tag since it's not styled anymore in newer
>   bootstrap versions
> * add 'form-label' to labels
> * use containers with 'mb-3' for form + buttons
> * use 'd-grid' container for button instead of 'btn-block'
> * add 'breadcrumb-item' where necessary
> 
> Since bootstrap 5 does not depend on jQuery anymore, use that chance to
> remove it here as dependency too. For that remove the 'button'
> and 'add_js' subs that were never actually used.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  debian/control                           |   3 +-
>  src/PVE/APIServer/AnyEvent.pm            |   3 +
>  src/PVE/APIServer/Formatter/Bootstrap.pm |  37 +------
>  src/PVE/APIServer/Formatter/HTML.pm      | 132 +++++++++++++----------
>  4 files changed, 84 insertions(+), 91 deletions(-)
> 
> diff --git a/debian/control b/debian/control
> index 0d0161e..4ce0368 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -15,8 +15,7 @@ Depends: libanyevent-http-perl,
>           libhttp-date-perl,
>           libhttp-message-perl,
>           libio-socket-ssl-perl,
> -         libjs-bootstrap,
> -         libjs-jquery,
> +         libjs-bootstrap5,
>           libjson-perl,
>           libnet-ip-perl,
>           libpve-common-perl (>= 8.0.2),
> diff --git a/src/PVE/APIServer/AnyEvent.pm b/src/PVE/APIServer/AnyEvent.pm
> index b71a9a5..9aeae2f 100644
> --- a/src/PVE/APIServer/AnyEvent.pm
> +++ b/src/PVE/APIServer/AnyEvent.pm
> @@ -2032,6 +2032,9 @@ sub new {

expanding the context a bit here:

 # add default dirs which includes jquery and bootstrap
 my $jsbase = '/usr/share/javascript';                 
 add_dirs($self->{dirs}, '/js/' => "$jsbase/");        

The jquery part would be wrong, and with the new location of the bootstrap file
we could evaluate if it's really required to expose all of /usr/share/javascript.
I mean, it does not really hurt, we disallow symlinks anyway and there is nothing
secretive in there nor do we interpret any files directly as code on the server,
so there is probably no big reason, but if we touch this it might be still worth
a quick evaluation.

>      my $glyphicons = '/usr/share/fonts/truetype/glyphicons/';
>      add_dirs($self->{dirs}, '/js/bootstrap/fonts/' => "$glyphicons");

but that font file and directory is provided by the fonts-glyphicons-halflings package,
the old libjs-bootstrap depends on that, the newer does not and thus quite likely
doesn't use it anymore. Can we drop it here? As on a fresh installation with this
patch applied it won't be reachable anyway.


besides that it look OK, albeit I only skimmed the remaining changes.

>  
> +    # libjs-bootstrap5 uses a different dir with symlinks
> +    add_dirs($self->{dirs}, '/js/bootstrap5/' => "/usr/share/bootstrap-html");
> +
>      # init inotify
>      PVE::INotify::inotify_init();
>  


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

Re: [pve-devel] [PATCH http-server 3/3] formatter: html: update to bootstrap 5

[Dominik Csapak]

On 6/4/25 19:33, Thomas Lamprecht wrote:
> Am 03.06.25 um 15:04 schrieb Dominik Csapak:
>> this makes a few changes necessary, but not too much:
>> * include the different directory for bootstrap5
>> * use different navbar markup
>> * different classes for navbar container + items
>> * add classes to pre tag since it's not styled anymore in newer
>>    bootstrap versions
>> * add 'form-label' to labels
>> * use containers with 'mb-3' for form + buttons
>> * use 'd-grid' container for button instead of 'btn-block'
>> * add 'breadcrumb-item' where necessary
>>
>> Since bootstrap 5 does not depend on jQuery anymore, use that chance to
>> remove it here as dependency too. For that remove the 'button'
>> and 'add_js' subs that were never actually used.
>>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>>   debian/control                           |   3 +-
>>   src/PVE/APIServer/AnyEvent.pm            |   3 +
>>   src/PVE/APIServer/Formatter/Bootstrap.pm |  37 +------
>>   src/PVE/APIServer/Formatter/HTML.pm      | 132 +++++++++++++----------
>>   4 files changed, 84 insertions(+), 91 deletions(-)
>>
>> diff --git a/debian/control b/debian/control
>> index 0d0161e..4ce0368 100644
>> --- a/debian/control
>> +++ b/debian/control
>> @@ -15,8 +15,7 @@ Depends: libanyevent-http-perl,
>>            libhttp-date-perl,
>>            libhttp-message-perl,
>>            libio-socket-ssl-perl,
>> -         libjs-bootstrap,
>> -         libjs-jquery,
>> +         libjs-bootstrap5,
>>            libjson-perl,
>>            libnet-ip-perl,
>>            libpve-common-perl (>= 8.0.2),
>> diff --git a/src/PVE/APIServer/AnyEvent.pm b/src/PVE/APIServer/AnyEvent.pm
>> index b71a9a5..9aeae2f 100644
>> --- a/src/PVE/APIServer/AnyEvent.pm
>> +++ b/src/PVE/APIServer/AnyEvent.pm
>> @@ -2032,6 +2032,9 @@ sub new {
> 
> expanding the context a bit here:
> 
>   # add default dirs which includes jquery and bootstrap
>   my $jsbase = '/usr/share/javascript';
>   add_dirs($self->{dirs}, '/js/' => "$jsbase/");
> 
> The jquery part would be wrong, and with the new location of the bootstrap file
> we could evaluate if it's really required to expose all of /usr/share/javascript.
> I mean, it does not really hurt, we disallow symlinks anyway and there is nothing
> secretive in there nor do we interpret any files directly as code on the server,
> so there is probably no big reason, but if we touch this it might be still worth
> a quick evaluation.
> 
>>       my $glyphicons = '/usr/share/fonts/truetype/glyphicons/';
>>       add_dirs($self->{dirs}, '/js/bootstrap/fonts/' => "$glyphicons");
> 
> but that font file and directory is provided by the fonts-glyphicons-halflings package,
> the old libjs-bootstrap depends on that, the newer does not and thus quite likely
> doesn't use it anymore. Can we drop it here? As on a fresh installation with this
> patch applied it won't be reachable anyway.
> 
> 
> besides that it look OK, albeit I only skimmed the remaining changes.

yes you're right, we probably can drop more folders here that I missed.
I'll check which ones we can drop and update the comments if necessary in the v2

> 
>>   
>> +    # libjs-bootstrap5 uses a different dir with symlinks
>> +    add_dirs($self->{dirs}, '/js/bootstrap5/' => "/usr/share/bootstrap-html");
>> +
>>       # init inotify
>>       PVE::INotify::inotify_init();
>>   



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

[pve-devel] partially-applied: [PATCH http-server 1/3] formatter: html: remove unused code

[Thomas Lamprecht]

On Tue, 03 Jun 2025 15:04:24 +0200, Dominik Csapak wrote:
> the console js code was added in
>  63307be (add generic formatter framework
> in January 2017 and marked to be removed (in the FIXME) with
>  6189d2e (Formatter/Bootstrap.pm; use configured cookie_name)
> a day later.
> 
> This was never used, so simply remove it.
> 
> [...]

Applied the first two patches already, thanks!

[1/3] formatter: html: remove unused code
      commit: 08f6effe2b787f544e0c8795692c5f13a95f0cb8
[2/3] formatter: html: fix logout button
      commit: f10efa82d0c884058fda86bd2859e5f6d8f4d239


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