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

From: Sebastián Monía <[email-masked]>
Date: Sat, 29 Mar 2025 08:07:00 -0400

Hi Eli, 
I didn’t send an email saying that I would do it last weekend, I thought "nah surely this week I get to it". And here we are :(

But yes I will! :) As soon as I get a chance. 

On Sat, Mar 29, 2025, at 7:16 AM, Eli Zaretskii wrote:
> Ping!  Sebastián, would you like to submit an updated patch which
> addresses my comments?
> 
> > Cc: [email-masked], [email-masked]
> > Date: Thu, 13 Mar 2025 10:20:41 +0200
> > From: Eli Zaretskii <[email-masked]>
> > 
> > > 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?
> > 
> > 
> > 
> > 
>