Subject: Re: bug#76982: 30.1; url-generic-parse-url cannot parse Windows file paths

From: Eli Zaretskii <[email-masked]>
Date: Thu, 13 Mar 2025 10:20:41 +0200

> Cc: [email-masked]
> From: Sebastián Monía
>  <[email-masked]>
> Date: Wed, 12 Mar 2025 15:23:08 -0400
> 
> Morgan Willcock <[email-masked]> writes:
> 
> >   (url-filename (url-generic-parse-url "file:///c:/windows"))
> >   => "/c:/windows"
> >
> >   (url-filename (url-generic-parse-url "c:/windows"))
> >   => "/windows"
> >
> > This is relevant in functions such as
> > mml-expand-html-into-multipart-related, where there is a check that the
> > file exists:
> >
> >   (when (and (null (url-type parsed))
> >              (not (zerop (length (url-filename parsed))))
> >              (file-exists-p (url-filename parsed)))
> >     ...)
> >
> > An example of a real world problem is being unable to reliably send HTML
> > mail when using Windows:
> >
> > https://github.com/jeremy-compostella/org-msg/issues/122
> 
> Attached a potential patch.

Thanks.

> >From 25c0cbeb1523e3fed3d3409465b6dcc99c967a38 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Sebasti=C3=A1n=20Mon=C3=ADa?=
>  <[email-masked]>
> Date: Wed, 12 Mar 2025 15:16:19 -0400
> Subject: [PATCH] url-parse.el: Parsing exceptions for Windows paths
>  (bug#76982)

Please add a ChangeLog-style list of functions that are modified and a
short description of each modification.

> 
> ---
>  lisp/url/url-parse.el | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/lisp/url/url-parse.el b/lisp/url/url-parse.el
> index 4c65721b83d..1dff66c6228 100644
> --- a/lisp/url/url-parse.el
> +++ b/lisp/url/url-parse.el
> @@ -83,7 +83,12 @@ url-recreate-url
>  	 ;; port is empty or if its value would be the same as that of
>  	 ;; the scheme's default."
>  	 (port (url-port-if-non-default urlobj))
> -	 (file (url-filename urlobj))
> +         ;; For Windows/DOS-like paths, `url-generic-parse-url' strips
> +         ;; the leading /, so we need to add it back (bug#76982)
> +	 (file (if (and (string= "file" type)
> +                        (string-match "^[A-Za-z]:[/\\]" (url-filename urlobj)))
> +                   (concat "/" (url-filename urlobj))
> +                 (url-filename urlobj)))
>  	 (frag (url-target urlobj)))
>      (concat (if type (concat type ":"))
>  	    (if (url-fullness urlobj) "//")

OK.

> @@ -210,6 +215,13 @@ url-generic-parse-url
>  
>            (if (and host (string-match "%[0-9][0-9]" host))
>                (setq host (url-unhex-string host)))
> +
> +          ;; For file:// URIs, if the path "looks like" Windows/DOS, it
> +          ;; makes sense to strip the leading slash (bug#76982)
> +          (when (and (string= "file" scheme)
> +                     (string-match "^/[A-Za-z]:[/\\]" file))
> +            (setq file (substring file 1)))
> +
>            (url-parse-make-urlobj scheme user pass host port file
>  				 fragment nil full))))))

Isn't it cleaner to go one more character forward here:

          ;; 3.2. Authority
          (when (looking-at "//")
            (setq full t)
            (forward-char 2)  <<<<<<<<<<<<<<<<<<<<
            (setq save-pos (point))
            (skip-chars-forward "^/?#")

if the conditions for Windows-style absolute file name are fulfilled?

Also, could we please have some simple tests for this?