Opened 7 years ago

Closed 6 years ago

#9162 closed defect (fixed)

wxPATH_NORM_CASE doesn't normalize case

Reported by: ma_boehm Owned by:
Priority: normal Milestone: 2.8.10
Component: wxMSW Version:
Keywords: wxFileName Cc: ma_boehm, neis
Blocked By: Blocking:
Patch: yes

Description

This is a simple patch to the problem described in
[ 1885241 ] wxPATH_NORM_CASE doesn't norm case.

It simply has been forgotten that the path returned from GetLongPath must be made lower case as well.

Attachments (4)

norm_case_patch.patch download (603 bytes) - added by ma_boehm 7 years ago.
norm_case_efficient.patch download (707 bytes) - added by neis 7 years ago.
slightly more efficient version
norm_case_patch3.patch download (1.6 KB) - added by ma_boehm 7 years ago.
an even better patch
testcase.zip download (5.1 KB) - added by ma_boehm 7 years ago.
Test case which reproduces the bug

Download all attachments as: .zip

Change History (17)

Changed 7 years ago by ma_boehm

Changed 7 years ago by neis

slightly more efficient version

comment:1 Changed 7 years ago by neis

I normally don't care much about such minor efficiency issues, but I really wonder if it's necessary to
make both the short and the long path lower case, i.e. I'd suggest the slightly more efficient version that I
attached, but I don't have a suitable test environment at hand, so maybe there's a drawback I'm
overlooking?

File Added: norm_case_efficient.patch

Changed 7 years ago by ma_boehm

an even better patch

comment:2 Changed 7 years ago by ma_boehm

OK, here is a new patch, which bases on your idea; and moreover i removed some further unnecessary lines of code: it is enough to make the path lower case once at the end of the Normalize function! Only thing i had to add there is that the directory entries in the m_dirs member variable also have to be made lower case.
File Added: norm_case_patch3.patch

comment:3 Changed 7 years ago by vadz

  • Keywords wxFileName added
  • Type set to build error

I wanted to apply this patch but I also wanted to reproduce this bug first, to be able to verify that the patch really fixes it and unfortunately I was unable to do so: the tests I added in r53636 pass without problems. I'm using trunk (but the code in 2.8 seems identical) and Windows 2003, maybe GetLongPathName() behaves differently under different systems? Which Windows version do you have, ma_boehm?

Anyhow, it would be great if someone who _can_ reproduce the bug could apply this patch. And I also think that we don't need MakeLower() calls just above if wxPATH_NORM_LONG is also specified.

Changed 7 years ago by ma_boehm

Test case which reproduces the bug

comment:4 Changed 7 years ago by ma_boehm

Ok, i'm using Windows *Vista*, and there is only *one* directory i know of which fails to be made lower case, that is C:\Users and all subdirectories as well. Windows Vista seems to treat this directory specially in GetLongPath, somehow.

I don't know whether there are other OS's with this directory (as user directory, i mean, because i think this is which makes Windows Vista treat that directory specially), but perhaps the error can be reproduced on other OS's with that directory as (special?) directory as well.
I added a test case, there are makefiles for borland+microsoft+mingw, and it assumes, that wxWidgets is installed in C:\wx (the files are actually taken from a project of mine), and you can control build by unicode=0/1 and debug=0/1.
I hope someone will reproduce this problem as well!

comment:5 Changed 7 years ago by ma_boehm

  • Status changed from new to need_feedback

comment:6 Changed 7 years ago by wxsite

  • Status changed from need_feedback to infoneeded_new

changed from old 'need_feedback' to new 'infoneeded_new'

comment:7 Changed 7 years ago by wxsite

changed from old 'need_feedback' to new 'infoneeded_new'

comment:8 Changed 7 years ago by ma_boehm

I don't know which operation system you use, but i ask you to to repeat your test (r53636) with the C:\Users (!!!) directory on Windows Vista (!!!), if possible; i'm sure that then you can reproduce the error as well. As i said before this is the *only* directory and OS i know where the error appears.

comment:9 Changed 7 years ago by vadz

  • Status changed from infoneeded_new to new

comment:10 Changed 7 years ago by vadz

  • Milestone set to 2.8.8
  • Resolution set to port to stable
  • Status changed from new to portneeded
  • Type changed from build error to defect

Ok, I could test it under Vista and the bug is indeed present there (even with C:\Program Files, not only C:\Users, although I added the test for the latter in r53732 just in case, too.

This probably should be backported into 2.8 too, unless we find something wrong with the new code.

Thanks for your patch!

comment:11 Changed 6 years ago by vadz

  • Milestone changed from 2.8.8 to 2.8.9
  • Summary changed from Patch for [ 1885241 ] wxPATH_NORM_CASE doesn't norm case to wxPATH_NORM_CASE doesn't normalize case

I feel we need to wait for a bit more to give more people a chance to test the new code.

comment:12 Changed 6 years ago by vadz

  • Milestone changed from 2.8.9 to 2.8.10

Did anybody test this patch with 2.8 by chance? If not, I'm afraid we'd have to postpone backporting it as I'm too afraid to break something here.

comment:13 Changed 6 years ago by VZ

  • Resolution changed from port to stable to fixed
  • Status changed from portneeded to closed

(In [58751]) do case normalization after long path one, this fixes the problem with incorrect paths case under Vista [backport of r53732 from trunk] (closes #9162)

Note: See TracTickets for help on using tickets.