os.stat() on windows doesn't consider relative symlink

[ocean-city]

BPO	12084
Nosy	@loewis, @birkenfeld, @jaraco, @vstinner, @ericvsmith, @tjguk, @briancurtin
Files	test.diff patch_v2.patch: just expanded from patches_v2.tar.gz patch_v2.txt: just expanded from patches_v2.tar.gz issue12084.diff issue12084_v2.diff issue12084_v3.diff: 3.2 patch smime.p7s issue12084_XP.diff quick1.patch quick2.patch quick3.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/briancurtin'
closed_at = <Date 2011-06-14.15:33:03.329>
created_at = <Date 2011-05-16.02:53:22.081>
labels = ['type-bug', 'OS-windows', 'release-blocker']
title = "os.stat() on windows doesn't consider relative symlink"
updated_at = <Date 2011-06-14.15:33:03.328>
user = 'https://bugs.python.org/ocean-city'

bugs.python.org fields:

activity = <Date 2011-06-14.15:33:03.328>
actor = 'brian.curtin'
assignee = 'brian.curtin'
closed = True
closed_date = <Date 2011-06-14.15:33:03.329>
closer = 'brian.curtin'
components = ['Windows']
creation = <Date 2011-05-16.02:53:22.081>
creator = 'ocean-city'
dependencies = []
files = ['22088', '22182', '22183', '22224', '22274', '22342', '22357', '22359', '22361', '22362', '22363']
hgrepos = []
issue_num = 12084
keywords = ['patch', 'needs review', '3.2regression']
message_count = 37.0
messages = ['136062', '136069', '136109', '136209', '136244', '136253', '136266', '136711', '136716', '136750', '137005', '137103', '137127', '137458', '137499', '137796', '137800', '137801', '137803', '137805', '137813', '137826', '137841', '138256', '138259', '138263', '138274', '138277', '138279', '138280', '138281', '138288', '138303', '138304', '138323', '138324', '138332']
nosy_count = 11.0
nosy_names = ['loewis', 'georg.brandl', 'jaraco', 'vstinner', 'ocean-city', 'eric.smith', 'tim.golden', 'nadeem.vawda', 'brian.curtin', 'santoso.wijaya', 'python-dev']
pr_nums = []
priority = 'release blocker'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue12084'
versions = ['Python 3.2', 'Python 3.3']

[ocean-city]

Hello. I noticed os.stat() on windows may traverse wrong path on relative symbolic when current working directory != the directory where symbolic link is in. This is because the relative path DeviceIoControl() returns is just passed to win32_xstat without converting to absolute path.

I'm sorry because I implemented this function, and it's hard for me to debug this because I don't have Vista/7. This patch uses GetFinalPathNameByHandle like original code does, plus should handle symlink to system locked file.

Can anyone test and commit this patch?

[birkenfeld]

It's quite a large patch... :)

I now own a new laptop that had Windows 7 preinstalled, so I'll try to get set up VC++ Express, and then test the patch. Still I'm nosying Martin as well.

For the future, it's much easier to just attach both files individually, this saves the need to download and unpack the archive.

[briancurtin]

Hirokazu contacted me directly with these patches a few days ago but I haven't been able to email him because his host's DNS is apparently down.

The tests in this patch do not end up testing anything, so we'll need to start with a proper test. In the brief look I had the other day, adding an except clause to the outer try/finally block catches something about a file not being available IIRC. I'll take a look this afternoon/evening.

[birkenfeld]

Brian, do you think you'll be able to finish this for 3.2.1?

If we do fix it, we'd need a second rc (not a problem for me).

[briancurtin]

I'm hoping to. I have time to work on it tonight and tomorrow night US/Chicago time and will keep you posted.

[jaraco]

Brian, I'm available to help with this tomorrow evening; let me know if you want to team up on it then.

[briancurtin]

With the patch applied, the new test fails along with test.test_os.WalkTests.test_traversal and test.test_os.Win32SymlinkTests.test_directory_link.

Overall, I agree that this doesn't work correctly. The patch, which is pretty large, breaks more than it fixes.

I'll see if I can make time during the day tomorrow to look further into the code, and I'll try to coordinate with Jason if possible.

(for anyone else looking at this, remember to run your command prompt as administrator)

[briancurtin]

Here's standalone patch which should cover this problem. The patch fails right now but succeeds if you apply it back to 652baf23c368 (the changeset before one of several changes around this code). I'll try to find the actual offending checkin and workout the differences to make a fix.

[briancurtin]

Ok, so it's 893b098929e7 where that test stops working.

[briancurtin]

Correction for msg136711 -- s/patch/test/g

[briancurtin]

Ok, so it's actually 0a1baa619171 that broke it, not sure how I came up with the other revision. In any case, it's too hairy to try and piece everything together across the numerous bug fixes, feature adds, and refactorings in this area in order to get back to a working state (plus you'll want to jump out the window if you try, believe me). I'm going to have a go at basically rewriting the os.stat Windows path tonight and tomorrow and see what that does.