Loading svg from memory using custom filename for base_uri

Loading svg from memory using custom filename for base_uri

[Evgeny Zajcev]

[-- Attachment #1: Type: text/plain, Size: 466 bytes --]

Currently, when SVG image is loaded with `svg_load` it first examines :file
value and only if :file is missing it looks for :data, using current
buffer's filename (if any) as base_uri

Would not it be better to check for :data first and if present, then use
:file as value for base_uri?  Falling back to current buffer filename in
case :file is missing

Unfortunately embedding raster images into svg with base64 encoding, using
`svg-embed` is slow.

Thanks

-- 
lg

[-- Attachment #2: Type: text/html, Size: 701 bytes --]

Re: Loading svg from memory using custom filename for base_uri

[Eli Zaretskii]

> From: Evgeny Zajcev <lg.zevlg@gmail.com>
> Date: Sat, 23 Feb 2019 01:01:02 +0300
> 
> Currently, when SVG image is loaded with `svg_load` it first examines :file value and only if :file is missing it
> looks for :data, using current buffer's filename (if any) as base_uri
> 
> Would not it be better to check for :data first and if present, then use :file as value for base_uri?  Falling back
> to current buffer filename in case :file is missing
> 
> Unfortunately embedding raster images into svg with base64 encoding, using `svg-embed` is slow.

I don't think I understand what you are saying here.  The test whether
an image spec specifies a file or not is just 2 lines of C:

  /* If IMG->spec specifies a file name, create a non-file spec from it.  */
  file_name = image_spec_value (img->spec, QCfile, NULL);
  if (STRINGP (file_name))
    {
      [load an image from file...]
    }
  /* Else its not a file, it's a lisp object.  Load the image from a
     lisp object rather than a file.  */
  else
    {
      [load an image from data...]
    }

And image_spec_value just walks a Lisp list looking for ':file':

  static Lisp_Object
  image_spec_value (Lisp_Object spec, Lisp_Object key, bool *found)
  {
    Lisp_Object tail;

    eassert (valid_image_p (spec));

    for (tail = XCDR (spec);
	 CONSP (tail) && CONSP (XCDR (tail));
	 tail = XCDR (XCDR (tail)))
      {
	if (EQ (XCAR (tail), key))
	  {
	    if (found)
	      *found = 1;
	    return XCAR (XCDR (tail));
	  }
      }

So I don't see how reversing the order would speed up SVG loading.
What did I miss?

Re: Loading svg from memory using custom filename for base_uri

[Evgeny Zajcev]

[-- Attachment #1: Type: text/plain, Size: 3171 bytes --]

сб, 23 февр. 2019 г. в 10:45, Eli Zaretskii <eliz@gnu.org>:

> > From: Evgeny Zajcev <lg.zevlg@gmail.com>
> > Date: Sat, 23 Feb 2019 01:01:02 +0300
> >
> > Currently, when SVG image is loaded with `svg_load` it first examines
> :file value and only if :file is missing it
> > looks for :data, using current buffer's filename (if any) as base_uri
> >
> > Would not it be better to check for :data first and if present, then use
> :file as value for base_uri?  Falling back
> > to current buffer filename in case :file is missing
> >
> > Unfortunately embedding raster images into svg with base64 encoding,
> using `svg-embed` is slow.
>
> I don't think I understand what you are saying here.  The test whether
> an image spec specifies a file or not is just 2 lines of C:
>
>
Ah, sorry, let me explain in more details.  SVG has ability embedding
raster images using "image" node with "xlink:href" attribute pointing to
the filename to embed.  rsvg library disallows absolute path names in
"xlin:href" due to security reasons.  To refer the filename with rsvg you
need to setup base_filename (aka base_uri) and then point to the file
relatively to this base_filename.  If you load .svg from filename, then
this filename is used as base_filename, but if you load SVG from memory you
need to specify base_uri explicitly.

`svg-embed` from svg.el does not bother with base_uri stuff and just embeds
raster images using base64 trick: it reads content of the file, encode it
with base64 and inserts encoded data into "image" node.  This is relative
slow thing to do.

To suppress this base64 trick, I want "xlink:href" to point directly to
filename, so I need some way to explicitly specify base_uri for rsvg to
use.  svg_load() from "image.c" uses `:file` as base_uri if loading from
filename, and current buffer's filename if loading from memory via `:data`.

I want to load svg `:data` from memory and specify base_uri, using image's
`:file` attribute.  This is not possible right now because `:file`
attribute is examined first in svg_load() function


  /* If IMG->spec specifies a file name, create a non-file spec from it.  */
>   file_name = image_spec_value (img->spec, QCfile, NULL);
>   if (STRINGP (file_name))
>     {
>       [load an image from file...]
>     }
>   /* Else its not a file, it's a lisp object.  Load the image from a
>      lisp object rather than a file.  */
>   else
>     {
>       [load an image from data...]
>     }
>
> And image_spec_value just walks a Lisp list looking for ':file':
>
>   static Lisp_Object
>   image_spec_value (Lisp_Object spec, Lisp_Object key, bool *found)
>   {
>     Lisp_Object tail;
>
>     eassert (valid_image_p (spec));
>
>     for (tail = XCDR (spec);
>          CONSP (tail) && CONSP (XCDR (tail));
>          tail = XCDR (XCDR (tail)))
>       {
>         if (EQ (XCAR (tail), key))
>           {
>             if (found)
>               *found = 1;
>             return XCAR (XCDR (tail));
>           }
>       }
>
> So I don't see how reversing the order would speed up SVG loading.
> What did I miss?
>


-- 
lg

[-- Attachment #2: Type: text/html, Size: 4162 bytes --]

Re: Loading svg from memory using custom filename for base_uri

[Evgeny Zajcev]

[-- Attachment #1.1: Type: text/plain, Size: 4277 bytes --]

пн, 25 февр. 2019 г. в 13:23, Evgeny Zajcev <lg.zevlg@gmail.com>:

>
>
> сб, 23 февр. 2019 г. в 10:45, Eli Zaretskii <eliz@gnu.org>:
>
>> > From: Evgeny Zajcev <lg.zevlg@gmail.com>
>> > Date: Sat, 23 Feb 2019 01:01:02 +0300
>> >
>> > Currently, when SVG image is loaded with `svg_load` it first examines
>> :file value and only if :file is missing it
>> > looks for :data, using current buffer's filename (if any) as base_uri
>> >
>> > Would not it be better to check for :data first and if present, then
>> use :file as value for base_uri?  Falling back
>> > to current buffer filename in case :file is missing
>> >
>> > Unfortunately embedding raster images into svg with base64 encoding,
>> using `svg-embed` is slow.
>>
>> I don't think I understand what you are saying here.  The test whether
>> an image spec specifies a file or not is just 2 lines of C:
>>
>>
> Ah, sorry, let me explain in more details.  SVG has ability embedding
> raster images using "image" node with "xlink:href" attribute pointing to
> the filename to embed.  rsvg library disallows absolute path names in
> "xlin:href" due to security reasons.  To refer the filename with rsvg you
> need to setup base_filename (aka base_uri) and then point to the file
> relatively to this base_filename.  If you load .svg from filename, then
> this filename is used as base_filename, but if you load SVG from memory you
> need to specify base_uri explicitly.
>
> `svg-embed` from svg.el does not bother with base_uri stuff and just
> embeds raster images using base64 trick: it reads content of the file,
> encode it with base64 and inserts encoded data into "image" node.  This is
> relative slow thing to do.
>
> To suppress this base64 trick, I want "xlink:href" to point directly to
> filename, so I need some way to explicitly specify base_uri for rsvg to
> use.  svg_load() from "image.c" uses `:file` as base_uri if loading from
> filename, and current buffer's filename if loading from memory via `:data`.
>
> I want to load svg `:data` from memory and specify base_uri, using image's
> `:file` attribute.  This is not possible right now because `:file`
> attribute is examined first in svg_load() function
>
>
>   /* If IMG->spec specifies a file name, create a non-file spec from it.
>> */
>>   file_name = image_spec_value (img->spec, QCfile, NULL);
>>   if (STRINGP (file_name))
>>     {
>>       [load an image from file...]
>>     }
>>   /* Else its not a file, it's a lisp object.  Load the image from a
>>      lisp object rather than a file.  */
>>   else
>>     {
>>       [load an image from data...]
>>     }
>>
>> And image_spec_value just walks a Lisp list looking for ':file':
>>
>>   static Lisp_Object
>>   image_spec_value (Lisp_Object spec, Lisp_Object key, bool *found)
>>   {
>>     Lisp_Object tail;
>>
>>     eassert (valid_image_p (spec));
>>
>>     for (tail = XCDR (spec);
>>          CONSP (tail) && CONSP (XCDR (tail));
>>          tail = XCDR (XCDR (tail)))
>>       {
>>         if (EQ (XCAR (tail), key))
>>           {
>>             if (found)
>>               *found = 1;
>>             return XCAR (XCDR (tail));
>>           }
>>       }
>>
>> So I don't see how reversing the order would speed up SVG loading.
>> What did I miss?
>>
>
`:file' property is hardly could be used for this, because it is used in
many different places beside svg, so I come up with next solution,
introducing `:base-uri' image property to explicitly set base_uri for
embedding images into svg

To test this feature, copy any .jpg image to /tmp/soviet.jpg and eval in
scratch

  (insert-image '(image :type svg :data "<svg width=\"100\" height=\"100\"
version=\"1.1\" xmlns=\"http://www.w3.org/2000/svg\" xmlns:xlink=\"
http://www.w3.org/1999/xlink\"> <image xlink:href=\"soviet.jpg\"
height=\"40\" width=\"40\" y=\"10\" x=\"10\"></image></svg>" :scale 1.0
:base-uri "/tmp/soviet.jpg" :width 100 :height 100 :ascent center))

svg with embedded image should be inserted.

To check where or not Emacs supports this feature use:

  (obarray-get c-keywords-obarray :base-uri)

This could be used to fallback to base64 encoding trick in older Emacs

Thanks

-- 
lg

[-- Attachment #1.2: Type: text/html, Size: 5735 bytes --]

[-- Attachment #2: 0001-Explicitly-specify-svg-base_uri-using-base-uri-image.patch --]
[-- Type: text/x-patch, Size: 1936 bytes --]

From 9a5ef4707502ed37c2bef677482c27db1dca78b6 Mon Sep 17 00:00:00 2001
From: Zajcev Evgeny <zevlg@yandex.ru>
Date: Thu, 3 Dec 2020 18:37:18 +0300
Subject: [PATCH] Explicitly specify svg base_uri using `:base-uri' image
 property

* src/image.c (svg_load): Check `:base-uri' image property to
  explicitly set base_uri for images embedded into SVG
---
 src/image.c | 5 ++++-
 src/xdisp.c | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/image.c b/src/image.c
index 5eb4132295..fa818d5446 100644
--- a/src/image.c
+++ b/src/image.c
@@ -9737,7 +9737,9 @@ svg_load (struct frame *f, struct image *img)
 	  image_error ("Invalid image data `%s'", data);
 	  return 0;
 	}
-      original_filename = BVAR (current_buffer, filename);
+      original_filename = image_spec_value (img->spec, QCbase_uri, NULL);
+      if (!STRINGP (original_filename))
+        original_filename = BVAR (current_buffer, filename);
       success_p = svg_load_image (f, img, SSDATA (data), SBYTES (data),
                                   (NILP (original_filename) ? NULL
 				   : SSDATA (original_filename)));
@@ -10002,6 +10004,7 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
   eassume (rsvg_handle);
 
   /* Set base_uri for properly handling referenced images (via 'href').
+     Can be explicitly specified using `:base_uri' image property.
      See rsvg bug 596114 - "image refs are relative to curdir, not .svg file"
      <https://gitlab.gnome.org/GNOME/librsvg/issues/33>. */
   if (filename)
diff --git a/src/xdisp.c b/src/xdisp.c
index 76ef420a36..51735b269d 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -34631,6 +34631,7 @@ syms_of_xdisp (void)
   DEFSYM (QCeval, ":eval");
   DEFSYM (QCpropertize, ":propertize");
   DEFSYM (QCfile, ":file");
+  DEFSYM (QCbase_uri, ":base-uri");
   DEFSYM (Qfontified, "fontified");
   DEFSYM (Qfontification_functions, "fontification-functions");
 
-- 
2.25.1

Re: Loading svg from memory using custom filename for base_uri

[Alan Third]

On Thu, Dec 03, 2020 at 06:47:37PM +0300, Evgeny Zajcev wrote:
> diff --git a/src/xdisp.c b/src/xdisp.c
> index 76ef420a36..51735b269d 100644
> --- a/src/xdisp.c
> +++ b/src/xdisp.c
> @@ -34631,6 +34631,7 @@ syms_of_xdisp (void)
>    DEFSYM (QCeval, ":eval");
>    DEFSYM (QCpropertize, ":propertize");
>    DEFSYM (QCfile, ":file");
> +  DEFSYM (QCbase_uri, ":base-uri");
>    DEFSYM (Qfontified, "fontified");
>    DEFSYM (Qfontification_functions, "fontification-functions");

Hi Evgeny, is there any reason this couldn't go in image.c? I think we
only need it when librsvg is compiled in too.

-- 
Alan Third

Re: Loading svg from memory using custom filename for base_uri

[lg.zevlg]

> 3 дек. 2020 г., в 19:17, Alan Third <alan@idiocy.org> написал(а):
> 
> On Thu, Dec 03, 2020 at 06:47:37PM +0300, Evgeny Zajcev wrote:
>> diff --git a/src/xdisp.c b/src/xdisp.c
>> index 76ef420a36..51735b269d 100644
>> --- a/src/xdisp.c
>> +++ b/src/xdisp.c
>> @@ -34631,6 +34631,7 @@ syms_of_xdisp (void)
>>   DEFSYM (QCeval, ":eval");
>>   DEFSYM (QCpropertize, ":propertize");
>>   DEFSYM (QCfile, ":file");
>> +  DEFSYM (QCbase_uri, ":base-uri");
>>   DEFSYM (Qfontified, "fontified");
>>   DEFSYM (Qfontification_functions, "fontification-functions");
> 
> Hi Evgeny, is there any reason this couldn't go in image.c? I think we
> only need it when librsvg is compiled in too.

You are right, this is svg only thing, I’ll fix

—
lg

Re: Loading svg from memory using custom filename for base_uri

[Alan Third]

On Thu, Dec 03, 2020 at 07:25:10PM +0300, lg.zevlg@gmail.com wrote:
> 
> > 3 дек. 2020 г., в 19:17, Alan Third <alan@idiocy.org> написал(а):
> > 
> > On Thu, Dec 03, 2020 at 06:47:37PM +0300, Evgeny Zajcev wrote:
> >> diff --git a/src/xdisp.c b/src/xdisp.c
> >> index 76ef420a36..51735b269d 100644
> >> --- a/src/xdisp.c
> >> +++ b/src/xdisp.c
> >> @@ -34631,6 +34631,7 @@ syms_of_xdisp (void)
> >>   DEFSYM (QCeval, ":eval");
> >>   DEFSYM (QCpropertize, ":propertize");
> >>   DEFSYM (QCfile, ":file");
> >> +  DEFSYM (QCbase_uri, ":base-uri");
> >>   DEFSYM (Qfontified, "fontified");
> >>   DEFSYM (Qfontification_functions, "fontification-functions");
> > 
> > Hi Evgeny, is there any reason this couldn't go in image.c? I think we
> > only need it when librsvg is compiled in too.
> 
> You are right, this is svg only thing, I’ll fix

I'm also wondering whether this is something that would be useful when
loading from a file and not just data? It might be considered a
security risk, I suppose?
-- 
Alan Third

Re: Loading svg from memory using custom filename for base_uri

[Evgeny Zajcev]

[-- Attachment #1: Type: text/plain, Size: 1536 bytes --]

чт, 3 дек. 2020 г. в 19:30, Alan Third <alan@idiocy.org>:

> On Thu, Dec 03, 2020 at 07:25:10PM +0300, lg.zevlg@gmail.com wrote:
> >
> > > 3 дек. 2020 г., в 19:17, Alan Third <alan@idiocy.org> написал(а):
> > >
> > > On Thu, Dec 03, 2020 at 06:47:37PM +0300, Evgeny Zajcev wrote:
> > >> diff --git a/src/xdisp.c b/src/xdisp.c
> > >> index 76ef420a36..51735b269d 100644
> > >> --- a/src/xdisp.c
> > >> +++ b/src/xdisp.c
> > >> @@ -34631,6 +34631,7 @@ syms_of_xdisp (void)
> > >>   DEFSYM (QCeval, ":eval");
> > >>   DEFSYM (QCpropertize, ":propertize");
> > >>   DEFSYM (QCfile, ":file");
> > >> +  DEFSYM (QCbase_uri, ":base-uri");
> > >>   DEFSYM (Qfontified, "fontified");
> > >>   DEFSYM (Qfontification_functions, "fontification-functions");
> > >
> > > Hi Evgeny, is there any reason this couldn't go in image.c? I think we
> > > only need it when librsvg is compiled in too.
> >
> > You are right, this is svg only thing, I’ll fix
>
> I'm also wondering whether this is something that would be useful when
> loading from a file and not just data? It might be considered a
> security risk, I suppose?
>

No risk, because `:base-uri` is part of image properties and not svg data.
And if one specified explicitly `:base-uri` then he knows what he is doing
and understands that loading an svg image could access files inside
`:base-uri`.

Having control over svg base_uri is a nice thing to have for both data and
file image specifiers.

I'll update the patch

-- 
lg

[-- Attachment #2: Type: text/html, Size: 2361 bytes --]

Re: Loading svg from memory using custom filename for base_uri

[Evgeny Zajcev]

[-- Attachment #1.1: Type: text/plain, Size: 1753 bytes --]

чт, 3 дек. 2020 г. в 19:54, Evgeny Zajcev <lg.zevlg@gmail.com>:

>
>
> чт, 3 дек. 2020 г. в 19:30, Alan Third <alan@idiocy.org>:
>
>> On Thu, Dec 03, 2020 at 07:25:10PM +0300, lg.zevlg@gmail.com wrote:
>> >
>> > > 3 дек. 2020 г., в 19:17, Alan Third <alan@idiocy.org> написал(а):
>> > >
>> > > On Thu, Dec 03, 2020 at 06:47:37PM +0300, Evgeny Zajcev wrote:
>> > >> diff --git a/src/xdisp.c b/src/xdisp.c
>> > >> index 76ef420a36..51735b269d 100644
>> > >> --- a/src/xdisp.c
>> > >> +++ b/src/xdisp.c
>> > >> @@ -34631,6 +34631,7 @@ syms_of_xdisp (void)
>> > >>   DEFSYM (QCeval, ":eval");
>> > >>   DEFSYM (QCpropertize, ":propertize");
>> > >>   DEFSYM (QCfile, ":file");
>> > >> +  DEFSYM (QCbase_uri, ":base-uri");
>> > >>   DEFSYM (Qfontified, "fontified");
>> > >>   DEFSYM (Qfontification_functions, "fontification-functions");
>> > >
>> > > Hi Evgeny, is there any reason this couldn't go in image.c? I think we
>> > > only need it when librsvg is compiled in too.
>> >
>> > You are right, this is svg only thing, I’ll fix
>>
>> I'm also wondering whether this is something that would be useful when
>> loading from a file and not just data? It might be considered a
>> security risk, I suppose?
>>
>
> No risk, because `:base-uri` is part of image properties and not svg
> data.  And if one specified explicitly `:base-uri` then he knows what he is
> doing and understands that loading an svg image could access files inside
> `:base-uri`.
>
> Having control over svg base_uri is a nice thing to have for both data and
> file image specifiers.
>
> I'll update the patch
>

Here is updated patch, with support for `:base-uri` for :file image spec as
well


-- 
lg

[-- Attachment #1.2: Type: text/html, Size: 2813 bytes --]

[-- Attachment #2: 0001-Explicitly-specify-svg-base_uri-using-base-uri-image.patch --]
[-- Type: text/x-patch, Size: 3364 bytes --]

From f2be66a6b37049efc44133bce4d895a88d226d3d Mon Sep 17 00:00:00 2001
From: Zajcev Evgeny <zevlg@yandex.ru>
Date: Thu, 3 Dec 2020 18:37:18 +0300
Subject: [PATCH] Explicitly specify svg base_uri using `:base-uri' image
 property

* src/image.c (svg_load): Check `:base-uri' image property to
  explicitly set base_uri for images embedded into SVG
---
 src/image.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/src/image.c b/src/image.c
index 5eb4132295..2abead46ed 100644
--- a/src/image.c
+++ b/src/image.c
@@ -9698,10 +9698,11 @@ init_svg_functions (void)
 svg_load (struct frame *f, struct image *img)
 {
   bool success_p = 0;
-  Lisp_Object file_name;
+  Lisp_Object file_name, base_uri;
 
   /* If IMG->spec specifies a file name, create a non-file spec from it.  */
   file_name = image_spec_value (img->spec, QCfile, NULL);
+  base_uri = image_spec_value (img->spec, QCbase_uri, NULL);
   if (STRINGP (file_name))
     {
       int fd;
@@ -9721,15 +9722,16 @@ svg_load (struct frame *f, struct image *img)
 	  return 0;
 	}
       /* If the file was slurped into memory properly, parse it.  */
-      success_p = svg_load_image (f, img, contents, size,
-				  SSDATA (ENCODE_FILE (file)));
+      if (!STRINGP (base_uri))
+        base_uri = ENCODE_FILE (file);
+      success_p = svg_load_image (f, img, contents, size, SSDATA (base_uri));
       xfree (contents);
     }
   /* Else it's not a file, it's a Lisp object.  Load the image from a
      Lisp object rather than a file.  */
   else
     {
-      Lisp_Object data, original_filename;
+      Lisp_Object data;
 
       data = image_spec_value (img->spec, QCdata, NULL);
       if (!STRINGP (data))
@@ -9737,10 +9739,10 @@ svg_load (struct frame *f, struct image *img)
 	  image_error ("Invalid image data `%s'", data);
 	  return 0;
 	}
-      original_filename = BVAR (current_buffer, filename);
+      if (!STRINGP (base_uri))
+        base_uri = BVAR (current_buffer, filename);
       success_p = svg_load_image (f, img, SSDATA (data), SBYTES (data),
-                                  (NILP (original_filename) ? NULL
-				   : SSDATA (original_filename)));
+                                  (NILP (base_uri) ? NULL : SSDATA (base_uri)));
     }
 
   return success_p;
@@ -9838,6 +9840,7 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
   eassume (rsvg_handle);
 
   /* Set base_uri for properly handling referenced images (via 'href').
+     Can be explicitly specified using `:base_uri' image property.
      See rsvg bug 596114 - "image refs are relative to curdir, not .svg file"
      <https://gitlab.gnome.org/GNOME/librsvg/issues/33>. */
   if (filename)
@@ -10002,6 +10005,7 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
   eassume (rsvg_handle);
 
   /* Set base_uri for properly handling referenced images (via 'href').
+     Can be explicitly specified using `:base_uri' image property.
      See rsvg bug 596114 - "image refs are relative to curdir, not .svg file"
      <https://gitlab.gnome.org/GNOME/librsvg/issues/33>. */
   if (filename)
@@ -10684,6 +10688,7 @@ syms_of_image (void)
 
 #if defined (HAVE_RSVG)
   DEFSYM (Qsvg, "svg");
+  DEFSYM (QCbase_uri, ":base-uri");
   add_image_type (Qsvg);
 #ifdef HAVE_NTGUI
   /* Other libraries used directly by svg code.  */
-- 
2.25.1

Re: Loading svg from memory using custom filename for base_uri

[Alan Third]

On Thu, Dec 03, 2020 at 08:50:15PM +0300, Evgeny Zajcev wrote:
> 
> Here is updated patch, with support for `:base-uri` for :file image spec as
> well

It looks good to me. There are two things I think we need. One is to
add an entry for :base-uri to the svg_format array in image.c, and
some documentation, if you fancy writing it?

Do we also need an entry in NEWS for this?
-- 
Alan Third

Re: Loading svg from memory using custom filename for base_uri

[Evgeny Zajcev]

[-- Attachment #1.1: Type: text/plain, Size: 587 bytes --]

чт, 3 дек. 2020 г. в 22:57, Alan Third <alan@idiocy.org>:

> On Thu, Dec 03, 2020 at 08:50:15PM +0300, Evgeny Zajcev wrote:
> >
> > Here is updated patch, with support for `:base-uri` for :file image spec
> as
> > well
>
> It looks good to me. There are two things I think we need. One is to
> add an entry for :base-uri to the svg_format array in image.c,

done!

and
> some documentation, if you fancy writing it?
>
> Documentation better to write when `svg-embed` will support this feature.

Do we also need an entry in NEWS for this?
>

Added

-- 
lg

[-- Attachment #1.2: Type: text/html, Size: 1296 bytes --]

[-- Attachment #2: 0001-Explicitly-specify-svg-base_uri-using-base-uri-image.patch --]
[-- Type: text/x-patch, Size: 4441 bytes --]

From 230359a4bd7ca75a28441f7e5fae6ef1440a8c1b Mon Sep 17 00:00:00 2001
From: Zajcev Evgeny <zevlg@yandex.ru>
Date: Thu, 3 Dec 2020 18:37:18 +0300
Subject: [PATCH] Explicitly specify svg base_uri using `:base-uri' image
 property

* src/image.c (svg_load): Check `:base-uri' image property to
  explicitly set base_uri for images embedded into SVG
---
 etc/NEWS    |  6 ++++++
 src/image.c | 20 +++++++++++++-------
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 2fb33e342e..493e456c6c 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -24,6 +24,12 @@ applies, and please also update docstrings as needed.
 \f
 * Installation Changes in Emacs 28.1
 
+---
+** Can explicitly specify base_uri for svg images.
+':base-uri' image property can be used to explicitly specify base_uri
+for embedded images into svg. ':base-uri' is supported for both file
+and data svg images.
+
 ** Cairo graphics library is now used by default if found.
 '--with-cairo' is now the default, if the appropriate development files
 are found by 'configure'.  Note that building with Cairo means using
diff --git a/src/image.c b/src/image.c
index 5eb4132295..89ebc60c6c 100644
--- a/src/image.c
+++ b/src/image.c
@@ -9472,6 +9472,7 @@ DEFUN ("imagemagick-types", Fimagemagick_types, Simagemagick_types, 0, 0, 0,
   {":type",		IMAGE_SYMBOL_VALUE,			1},
   {":data",		IMAGE_STRING_VALUE,			0},
   {":file",		IMAGE_STRING_VALUE,			0},
+  {":base-uri",		IMAGE_STRING_VALUE,			0},
   {":ascent",		IMAGE_ASCENT_VALUE,			0},
   {":margin",		IMAGE_NON_NEGATIVE_INTEGER_VALUE_OR_PAIR, 0},
   {":relief",		IMAGE_INTEGER_VALUE,			0},
@@ -9698,10 +9699,11 @@ init_svg_functions (void)
 svg_load (struct frame *f, struct image *img)
 {
   bool success_p = 0;
-  Lisp_Object file_name;
+  Lisp_Object file_name, base_uri;
 
   /* If IMG->spec specifies a file name, create a non-file spec from it.  */
   file_name = image_spec_value (img->spec, QCfile, NULL);
+  base_uri = image_spec_value (img->spec, QCbase_uri, NULL);
   if (STRINGP (file_name))
     {
       int fd;
@@ -9721,15 +9723,16 @@ svg_load (struct frame *f, struct image *img)
 	  return 0;
 	}
       /* If the file was slurped into memory properly, parse it.  */
-      success_p = svg_load_image (f, img, contents, size,
-				  SSDATA (ENCODE_FILE (file)));
+      if (!STRINGP (base_uri))
+        base_uri = ENCODE_FILE (file);
+      success_p = svg_load_image (f, img, contents, size, SSDATA (base_uri));
       xfree (contents);
     }
   /* Else it's not a file, it's a Lisp object.  Load the image from a
      Lisp object rather than a file.  */
   else
     {
-      Lisp_Object data, original_filename;
+      Lisp_Object data;
 
       data = image_spec_value (img->spec, QCdata, NULL);
       if (!STRINGP (data))
@@ -9737,10 +9740,10 @@ svg_load (struct frame *f, struct image *img)
 	  image_error ("Invalid image data `%s'", data);
 	  return 0;
 	}
-      original_filename = BVAR (current_buffer, filename);
+      if (!STRINGP (base_uri))
+        base_uri = BVAR (current_buffer, filename);
       success_p = svg_load_image (f, img, SSDATA (data), SBYTES (data),
-                                  (NILP (original_filename) ? NULL
-				   : SSDATA (original_filename)));
+                                  (NILP (base_uri) ? NULL : SSDATA (base_uri)));
     }
 
   return success_p;
@@ -9838,6 +9841,7 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
   eassume (rsvg_handle);
 
   /* Set base_uri for properly handling referenced images (via 'href').
+     Can be explicitly specified using `:base_uri' image property.
      See rsvg bug 596114 - "image refs are relative to curdir, not .svg file"
      <https://gitlab.gnome.org/GNOME/librsvg/issues/33>. */
   if (filename)
@@ -10002,6 +10006,7 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
   eassume (rsvg_handle);
 
   /* Set base_uri for properly handling referenced images (via 'href').
+     Can be explicitly specified using `:base_uri' image property.
      See rsvg bug 596114 - "image refs are relative to curdir, not .svg file"
      <https://gitlab.gnome.org/GNOME/librsvg/issues/33>. */
   if (filename)
@@ -10684,6 +10689,7 @@ syms_of_image (void)
 
 #if defined (HAVE_RSVG)
   DEFSYM (Qsvg, "svg");
+  DEFSYM (QCbase_uri, ":base-uri");
   add_image_type (Qsvg);
 #ifdef HAVE_NTGUI
   /* Other libraries used directly by svg code.  */
-- 
2.25.1

Re: Loading svg from memory using custom filename for base_uri

[Evgeny Zajcev]

[-- Attachment #1.1: Type: text/plain, Size: 737 bytes --]

пт, 4 дек. 2020 г. в 01:11, Evgeny Zajcev <lg.zevlg@gmail.com>:

>
>
> чт, 3 дек. 2020 г. в 22:57, Alan Third <alan@idiocy.org>:
>
>> On Thu, Dec 03, 2020 at 08:50:15PM +0300, Evgeny Zajcev wrote:
>> >
>> > Here is updated patch, with support for `:base-uri` for :file image
>> spec as
>> > well
>>
>> It looks good to me. There are two things I think we need. One is to
>> add an entry for :base-uri to the svg_format array in image.c,
>
> done!
>
> and
>> some documentation, if you fancy writing it?
>>
>> Documentation better to write when `svg-embed` will support this feature.
>

I wrote a new function `svg-embed-base-uri-image' to do the job, and wrote
documentation for it.



-- 
lg

[-- Attachment #1.2: Type: text/html, Size: 1599 bytes --]

[-- Attachment #2: 0001-Explicitly-specify-svg-base_uri-using-base-uri-image.patch --]
[-- Type: text/x-patch, Size: 6913 bytes --]

From ac2f4fe1ec8b2d848d815322ba683f99c7db2e36 Mon Sep 17 00:00:00 2001
From: Zajcev Evgeny <zevlg@yandex.ru>
Date: Thu, 3 Dec 2020 18:37:18 +0300
Subject: [PATCH] Explicitly specify svg base_uri using `:base-uri' image
 property

* src/image.c (svg_load): Check `:base-uri' image property to
  explicitly set base_uri for images embedded into SVG

* lisp/svg.el (svg-embed-base-uri-image): New function to embed images
  located relative to images `:base-uri'
---
 doc/lispref/display.texi | 20 ++++++++++++++++++++
 etc/NEWS                 | 11 +++++++++++
 lisp/svg.el              | 13 +++++++++++++
 src/image.c              | 20 +++++++++++++-------
 4 files changed, 57 insertions(+), 7 deletions(-)

diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index f86baf5936..0038a5c58d 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -5898,6 +5898,26 @@ SVG Images
 @end lisp
 @end defun
 
+@defun svg-embed-base-uri-image svg relative-filename &rest args
+To @var{svg} add an embedded (raster) image placed at
+@var{relative-filename}.  @var{relative-filename} is searched inside
+@code{file-name-directory} of the @code{:base-uri} svg image property.
+This makes embedding large images work very fast.
+
+@lisp
+;; Embeding /tmp/subdir/rms.jpg and /tmp/another/rms.jpg
+(svg-embed-base-uri-image svg "subdir/rms.jpg"
+           :width "100px" :height "100px"
+           :x "50px" :y "75px")
+(svg-embed-base-uri-image svg "another/rms.jpg"
+           :width "100px" :height "100px"
+           :x "75px" :y "50px")
+(svg-image svg :scale 1.0
+           :base-uri "/tmp/dummy"
+           :width 175 :height 175)
+@end lisp
+@end defun
+
 @defun svg-clip-path svg &rest args
 Add a clipping path to @var{svg}.  If applied to a shape via the
 @var{:clip-path} property, parts of that shape which lie outside of
diff --git a/etc/NEWS b/etc/NEWS
index 2fb33e342e..392b81f03c 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -24,6 +24,17 @@ applies, and please also update docstrings as needed.
 \f
 * Installation Changes in Emacs 28.1
 
+** Can explicitly specify base_uri for svg images.
+':base-uri' image property can be used to explicitly specify base_uri
+for embedded images into svg. ':base-uri' is supported for both file
+and data svg images.
+
++++
+** 'svg-embed-base-uri-image' added to embed images
+'svg-embed-base-uri-image' can be used to embed images located
+relatively to 'file-name-directory' of the ':base-uri' svg image property.
+This works much faster then 'svg-embed'.
+
 ** Cairo graphics library is now used by default if found.
 '--with-cairo' is now the default, if the appropriate development files
 are found by 'configure'.  Note that building with Cairo means using
diff --git a/lisp/svg.el b/lisp/svg.el
index eeb945f53b..6896e0088b 100644
--- a/lisp/svg.el
+++ b/lisp/svg.el
@@ -184,6 +184,19 @@ svg-embed
     `((xlink:href . ,(svg--image-data image image-type datap))
       ,@(svg--arguments svg args)))))
 
+(defun svg-embed-base-uri-image (svg relative-filename &rest args)
+  "Insert image placed at RELATIVE-FILENAME into the SVG structure.
+RELATIVE-FILENAME will be searched in `file-name-directory' of the
+image's `:base-uri' property.  If `:base-uri' is not specified for the
+image, then embedding won't work. Embedding large images using this
+function is much faster, then `svg-embed'."
+  (svg--append
+   svg
+   (dom-node
+    'image
+    `((xlink:href . ,relative-filename)
+      ,@(svg--arguments svg args)))))
+
 (defun svg-text (svg text &rest args)
   "Add TEXT to SVG."
   (svg--append
diff --git a/src/image.c b/src/image.c
index 5eb4132295..89ebc60c6c 100644
--- a/src/image.c
+++ b/src/image.c
@@ -9472,6 +9472,7 @@ DEFUN ("imagemagick-types", Fimagemagick_types, Simagemagick_types, 0, 0, 0,
   {":type",		IMAGE_SYMBOL_VALUE,			1},
   {":data",		IMAGE_STRING_VALUE,			0},
   {":file",		IMAGE_STRING_VALUE,			0},
+  {":base-uri",		IMAGE_STRING_VALUE,			0},
   {":ascent",		IMAGE_ASCENT_VALUE,			0},
   {":margin",		IMAGE_NON_NEGATIVE_INTEGER_VALUE_OR_PAIR, 0},
   {":relief",		IMAGE_INTEGER_VALUE,			0},
@@ -9698,10 +9699,11 @@ init_svg_functions (void)
 svg_load (struct frame *f, struct image *img)
 {
   bool success_p = 0;
-  Lisp_Object file_name;
+  Lisp_Object file_name, base_uri;
 
   /* If IMG->spec specifies a file name, create a non-file spec from it.  */
   file_name = image_spec_value (img->spec, QCfile, NULL);
+  base_uri = image_spec_value (img->spec, QCbase_uri, NULL);
   if (STRINGP (file_name))
     {
       int fd;
@@ -9721,15 +9723,16 @@ svg_load (struct frame *f, struct image *img)
 	  return 0;
 	}
       /* If the file was slurped into memory properly, parse it.  */
-      success_p = svg_load_image (f, img, contents, size,
-				  SSDATA (ENCODE_FILE (file)));
+      if (!STRINGP (base_uri))
+        base_uri = ENCODE_FILE (file);
+      success_p = svg_load_image (f, img, contents, size, SSDATA (base_uri));
       xfree (contents);
     }
   /* Else it's not a file, it's a Lisp object.  Load the image from a
      Lisp object rather than a file.  */
   else
     {
-      Lisp_Object data, original_filename;
+      Lisp_Object data;
 
       data = image_spec_value (img->spec, QCdata, NULL);
       if (!STRINGP (data))
@@ -9737,10 +9740,10 @@ svg_load (struct frame *f, struct image *img)
 	  image_error ("Invalid image data `%s'", data);
 	  return 0;
 	}
-      original_filename = BVAR (current_buffer, filename);
+      if (!STRINGP (base_uri))
+        base_uri = BVAR (current_buffer, filename);
       success_p = svg_load_image (f, img, SSDATA (data), SBYTES (data),
-                                  (NILP (original_filename) ? NULL
-				   : SSDATA (original_filename)));
+                                  (NILP (base_uri) ? NULL : SSDATA (base_uri)));
     }
 
   return success_p;
@@ -9838,6 +9841,7 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
   eassume (rsvg_handle);
 
   /* Set base_uri for properly handling referenced images (via 'href').
+     Can be explicitly specified using `:base_uri' image property.
      See rsvg bug 596114 - "image refs are relative to curdir, not .svg file"
      <https://gitlab.gnome.org/GNOME/librsvg/issues/33>. */
   if (filename)
@@ -10002,6 +10006,7 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
   eassume (rsvg_handle);
 
   /* Set base_uri for properly handling referenced images (via 'href').
+     Can be explicitly specified using `:base_uri' image property.
      See rsvg bug 596114 - "image refs are relative to curdir, not .svg file"
      <https://gitlab.gnome.org/GNOME/librsvg/issues/33>. */
   if (filename)
@@ -10684,6 +10689,7 @@ syms_of_image (void)
 
 #if defined (HAVE_RSVG)
   DEFSYM (Qsvg, "svg");
+  DEFSYM (QCbase_uri, ":base-uri");
   add_image_type (Qsvg);
 #ifdef HAVE_NTGUI
   /* Other libraries used directly by svg code.  */
-- 
2.25.1

Re: Loading svg from memory using custom filename for base_uri

[lg.zevlg]

[-- Attachment #1: Type: text/plain, Size: 1014 bytes --]


> 4 дек. 2020 г., в 02:02, Evgeny Zajcev <lg.zevlg@gmail.com> написал(а):
> 
> 
> 
> 
> пт, 4 дек. 2020 г. в 01:11, Evgeny Zajcev <lg.zevlg@gmail.com>:
>> 
>> 
>> чт, 3 дек. 2020 г. в 22:57, Alan Third <alan@idiocy.org>:
>>>> On Thu, Dec 03, 2020 at 08:50:15PM +0300, Evgeny Zajcev wrote:
>>>> > 
>>>> > Here is updated patch, with support for `:base-uri` for :file image spec as
>>>> > well
>>>> 
>>>> It looks good to me. There are two things I think we need. One is to
>>>> add an entry for :base-uri to the svg_format array in image.c,
>>> done!
>>> 
>>> and
>>> some documentation, if you fancy writing it?
>>> 
>> Documentation better to write when `svg-embed` will support this feature.
> 
> I wrote a new function `svg-embed-base-uri-image' to do the job, and wrote documentation for it.
> 

JFYI: I’ve been using Emacs with this patch applied for one week, utilizing svg file embeding heavily, works as expected without any issues

-- 
lg


[-- Attachment #2: Type: text/html, Size: 2094 bytes --]

Re: Loading svg from memory using custom filename for base_uri

[Alan Third]

On Sat, Dec 12, 2020 at 08:46:10AM +0300, lg.zevlg@gmail.com wrote:
> 
> > 4 дек. 2020 г., в 02:02, Evgeny Zajcev <lg.zevlg@gmail.com> написал(а):
> > 
> > 
> > 
> > 
> > пт, 4 дек. 2020 г. в 01:11, Evgeny Zajcev <lg.zevlg@gmail.com>:
> >> 
> >> 
> >> чт, 3 дек. 2020 г. в 22:57, Alan Third <alan@idiocy.org>:
> >>>> On Thu, Dec 03, 2020 at 08:50:15PM +0300, Evgeny Zajcev wrote:
> >>>> > 
> >>>> > Here is updated patch, with support for `:base-uri` for :file image spec as
> >>>> > well
> >>>> 
> >>>> It looks good to me. There are two things I think we need. One is to
> >>>> add an entry for :base-uri to the svg_format array in image.c,
> >>> done!
> >>> 
> >>> and
> >>> some documentation, if you fancy writing it?
> >>> 
> >> Documentation better to write when `svg-embed` will support this feature.
> > 
> > I wrote a new function `svg-embed-base-uri-image' to do the job, and wrote documentation for it.
> > 
> 
> JFYI: I’ve been using Emacs with this patch applied for one week,
> utilizing svg file embeding heavily, works as expected without any
> issues

Thanks. I've pushed it to the master branch.
-- 
Alan Third

Re: Loading svg from memory using custom filename for base_uri

[Evgeny Zajcev]

[-- Attachment #1: Type: text/plain, Size: 1514 bytes --]

сб, 12 дек. 2020 г. в 13:45, Alan Third <alan@idiocy.org>:

> On Sat, Dec 12, 2020 at 08:46:10AM +0300, lg.zevlg@gmail.com wrote:
> >
> > > 4 дек. 2020 г., в 02:02, Evgeny Zajcev <lg.zevlg@gmail.com>
> написал(а):
> > >
> > > 
> > >
> > >
> > > пт, 4 дек. 2020 г. в 01:11, Evgeny Zajcev <lg.zevlg@gmail.com>:
> > >>
> > >>
> > >> чт, 3 дек. 2020 г. в 22:57, Alan Third <alan@idiocy.org>:
> > >>>> On Thu, Dec 03, 2020 at 08:50:15PM +0300, Evgeny Zajcev wrote:
> > >>>> >
> > >>>> > Here is updated patch, with support for `:base-uri` for :file
> image spec as
> > >>>> > well
> > >>>>
> > >>>> It looks good to me. There are two things I think we need. One is to
> > >>>> add an entry for :base-uri to the svg_format array in image.c,
> > >>> done!
> > >>>
> > >>> and
> > >>> some documentation, if you fancy writing it?
> > >>>
> > >> Documentation better to write when `svg-embed` will support this
> feature.
> > >
> > > I wrote a new function `svg-embed-base-uri-image' to do the job, and
> wrote documentation for it.
> > >
> >
> > JFYI: I’ve been using Emacs with this patch applied for one week,
> > utilizing svg file embeding heavily, works as expected without any
> > issues
>
> Thanks. I've pushed it to the master branch.
>

Just interested, why not the latest patch with `svg-embed-base-uri-image`
function implemented and documentation written?

That latest patch, also adds :base-uri to svg_format spec

-- 
lg

[-- Attachment #2: Type: text/html, Size: 2493 bytes --]

Re: Loading svg from memory using custom filename for base_uri

[Alan Third]

On Sat, Dec 12, 2020 at 02:52:16PM +0300, Evgeny Zajcev wrote:
> сб, 12 дек. 2020 г. в 13:45, Alan Third <alan@idiocy.org>:
> > Thanks. I've pushed it to the master branch.
> >
> 
> Just interested, why not the latest patch with `svg-embed-base-uri-image`
> function implemented and documentation written?
> 
> That latest patch, also adds :base-uri to svg_format spec

Apologies, that was my mistake. I pushed the wrong one.

I've reverted it and pushed the new one.
-- 
Alan Third

Re: Loading svg from memory using custom filename for base_uri

[Vasilij Schneidermann]

[-- Attachment #1: Type: text/plain, Size: 580 bytes --]

> I'm also wondering whether this is something that would be useful when
> loading from a file and not just data? It might be considered a
> security risk, I suppose?

The examples in <https://debbugs.gnu.org/cgi/bugreport.cgi?bug=19373>
show files relying on a correctly set base-uri to work. There might be a
security risk if images are included that shouldn't be. Browsers
typically rely on Same-Origin Policy to shield off that risk (for
example a file:/// URL may only include other file:/// URLs), but it's a
heavy-handed solution and requires extra care to avoid bypasses.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: Loading svg from memory using custom filename for base_uri

[Vasilij Schneidermann]

[-- Attachment #1: Type: text/plain, Size: 454 bytes --]

> `:file' property is hardly could be used for this, because it is used in
> many different places beside svg, so I come up with next solution,
> introducing `:base-uri' image property to explicitly set base_uri for
> embedding images into svg

Thank you very much! I've attempted getting this working before, but
gave up on it. See also <https://debbugs.gnu.org/cgi/bugreport.cgi?bug=19373>
for the code that ended up in the current version of image.c.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]