bugGNU Octave - Bugs: bug #64462, fileparts.m mangles relative paths...

 
 

bug #64462: fileparts.m mangles relative paths on Windows drives

Submitter:  Philip Nienhuis <philipnienhuis>
Submitted:  Sun 23 Jul 2023 08:48:15 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  Philip Nienhuis Open/Closed:  * Closed
Release:  * 8.2.0 Operating System:  * Microsoft Windows
Fixed Release:  8.3.0 Planned Release:  8.3.0
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 17 Aug 2023 03:50:14 PM UTC, comment #6: 

Octave 8.3.0 containing this change has be released. With it, I see the following:

>> [a, b, c] = fileparts ('c:fntests.log')
a = c:
b = fntests
c = .log


Closing as fixed.

Markus Mützel <mmuetzel>
Group administrator
Fri 04 Aug 2023 06:24:56 PM UTC, comment #5: 

Thanks. I pushed your patch with very minor changes to the stable branch:
https://hg.savannah.gnu.org/hgweb/octave/rev/dc89f561451a

After that I followed up with some unrelated changes to the self tests and added a couple of new ones.
Instead of combining the test into a single assertion, I split them into multiple simpler ones. That would probably make it easier from the output of the test suite which part of the output is failing (if that should ever happen).

Marking as ready for test.

Markus Mützel <mmuetzel>
Group administrator
Sat 29 Jul 2023 02:17:40 PM UTC, comment #4: 

Thanks, all good suggestions.

Revised patch attached. I've added another BIST in it.

(file #54994)

Philip Nienhuis <philipnienhuis>
Group Member
Fri 28 Jul 2023 06:15:06 PM UTC, comment #3: 

Also, we usually use `%!testif ; ispc ()` instead of `%!testif WINDOWS_H`. And a reference to this bug report at the test would be nice. I.e., `%!testif ; ispc () <*64462>`.

Nit: Please, start sentences in the commit message with a capital letter and end them with a full stop.

Markus Mützel <mmuetzel>
Group administrator
Fri 28 Jul 2023 06:10:51 PM UTC, comment #2: 

Does that still need checks whether filename is at least two characters long before trying to index into it?

Markus Mützel <mmuetzel>
Group administrator
Sun 23 Jul 2023 08:49:59 PM UTC, comment #1: 

cset attached

(file #54958)

Philip Nienhuis <philipnienhuis>
Group Member
Sun 23 Jul 2023 08:48:15 PM UTC, original submission:  

A Windows-specific issue. Short way to reproduce:

>> [a, b, c] = fileparts ('c:fntests.log')
a =
b = c:fntests
c = .log

where the actual path ('c:') is somewhere on the current directory on drive C: (not the iroot per se). Note: It is a relative path on that drive.
The bug is that the fileparts function happily absorbs the drive letter and colon into the filename (output arg #2) rather than the directory (output arg #1). If the resulting filename is used later on, errors will result.

Now, c:fntests.log is a perfectly valid filename on Windows. More to read here:
https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file
and especially the stanza on that page under the heading 'Fully Qualified vs. Relative Paths':
"If a file name begins with only a disk designator but not the backslash after the colon, it is interpreted as a relative path to the current directory on the drive with the specified letter."
While that web page URL suggests it refers to 32-bit Windows, AFAICS this concept is still working on 64-bit Windows as well. E.g. Powershell can still work with relative paths on other drive (-letters); although it does silently insert a backslash (filesep) after the colon if you feed it a filename like 'f:somefile.txt'.

Filenames like 'C:somefile.ext' are not accepted when entered verbatim in the Open File dialog panes of Windows GUI programs.

I'm unsure about the best way to tackle this. IMO the best way would be to "properly" move the drive letter+colon and return it in output arg #1, cf. a patch I'll upload once I have a bug number. Although maybe the path part isn't that much usable, at least the actual filename will be parsed correctly. But fileparts.m doesn't check for validity/existence of its output anyway.

Philip Nienhuis <philipnienhuis>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #54994:  bug#64462_fileparts_v2.cset added by philipnienhuis (1KiB - application/octet-stream)
file #54958:  bug#64462_fileparts.cset added by philipnienhuis (1KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by philipnienhuis (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-08-17 mmuetzel StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2023-08-04 mmuetzel StatusPatch Submitted Ready For Test
        Releasedev 8.2.0
        Fixed ReleaseNone 8.3.0
        Planned ReleaseNone 8.3.0
    2023-07-29 philipnienhuis Attached File- Added bug#64462_fileparts_v2.cset, #54994
    2023-07-23 philipnienhuis Attached File- Added bug#64462_fileparts.cset, #54958
        StatusNone Patch Submitted

    Back to the top

    Powered by Savane 3.13-0329.
    Corresponding source code