Skip to content

rename fails on Windows PHP 8.1 if the target file is being executed #7910

@johnstevenson

Description

@johnstevenson
Contributor

Description

The following code:

<?php
// file: bug.php

$content = file_get_contents(__FILE__);
file_put_contents('newfile.php', $content."\n//");
var_dump(rename('newfile.php', __FILE__));

Resulted in this output:

PHP Warning:  rename(newfile.php,C:\Users\John\projects\test\bug.php): Access is denied (code: 5) in C:\Users\John\projects\test\bug.php on line 6

Warning: rename(newfile.php,C:\Users\John\projects\test\bug.php): Access is denied (code: 5) in C:\Users\John\projects\test\bug.php on line 6
bool(false)

But I expected this output instead:

bool(true)

This code mimics how Composer self updates (https://getcomposer.org/doc/03-cli.md#self-update-selfupdate-) and has obviously worked for many years, but is broken on Windows PHP 8.1: composer/composer#10444

Note that on Composer 2 we now use copy on Windows (to get around potential permission issues in UAC protected locations) and this still works on PHP 8.1, but Composer 1 uses rename (as do all non-Windows platforms on all Composer versions).

Fortunately, this appears to be a Windows only thing, as per this demo: https://github.com/johnstevenson/php-rename-bug/actions/runs/1669358275

PHP Version

PHP 8.1.0

Operating System

Windows 10

Activity

cmb69

cmb69 commented on Jan 7, 2022

@cmb69
Member

The regression is apparently caused by c732ab4. I shall have a look.

self-assigned this
on Jan 7, 2022
johnstevenson

johnstevenson commented on Jan 8, 2022

@johnstevenson
ContributorAuthor

Hey thanks. It seems like the file is locked, because cmd.exe move also fails when we call it as part of our UAC elevation stuff.

cmb69

cmb69 commented on Jan 8, 2022

@cmb69
Member

The file is not locked in a strict sense, but the handle is kept open after compilation, and that prevents the file to be renamed on Windows.

johnstevenson

johnstevenson commented on Jan 8, 2022

@johnstevenson
ContributorAuthor

Ah, so presumably the file has not been opened with FILE_SHARE_DELETE access.

cmb69

cmb69 commented on Jan 8, 2022

@cmb69
Member

The file is opened in FILE_SHARE_DELETE mode, but apparently, moving an opened source file is possible, while moving to an opened destination file is not:

C:\php-sdk\phpdev\vs16\x64
$ echo foo>foo

C:\php-sdk\phpdev\vs16\x64
$ echo bar>bar

C:\php-sdk\phpdev\vs16\x64
$ type movefile.c
#include <Windows.h>

int main(void)
{
    HANDLE file = CreateFileA(
        "bar",
        GENERIC_READ | GENERIC_WRITE,
        FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
        NULL,
        OPEN_EXISTING,
        FILE_ATTRIBUTE_NORMAL,
        NULL);

    BOOL res = MoveFileExA(
        "foo",
        "bar",
        MOVEFILE_REPLACE_EXISTING | MOVEFILE_COPY_ALLOWED);
    printf("%d", res);
    return 0;
}

C:\php-sdk\phpdev\vs16\x64
$ cl movefile.c
Microsoft (R) C/C++-Optimierungscompiler Version 19.29.30138 für x64
Copyright (C) Microsoft Corporation. Alle Rechte vorbehalten.

movefile.c
Microsoft (R) Incremental Linker Version 14.29.30138.0
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:movefile.exe
movefile.obj

C:\php-sdk\phpdev\vs16\x64
$ movefile.exe
0

C:\php-sdk\phpdev\vs16\x64
$ type foo
foo

C:\php-sdk\phpdev\vs16\x64
$ type bar
bar

I think we either need to close the script file right after compilation, or accept that this is just not supported on Windows. I'd prefer to do the former, unless there is a good reason to keep the script file open.

johnstevenson

johnstevenson commented on Jan 8, 2022

@johnstevenson
ContributorAuthor

Thanks for the demo. That makes sense (I should have thought about it a bit more).

I'd prefer to do the former, unless there is a good reason to keep the script file open.

Let's hope there isn't. I appreciate your time and involvement in this. Moving to Github issues has made PHP bugs a lot more interesting and compelling - for me anyway.

added a commit that references this issue on Jan 11, 2022
e996c94

9 remaining items

Voltra

Voltra commented on Feb 7, 2023

@Voltra

This also happens (sometimes) when files are being cached using Symfony or Laravel's default cache and filesystem adapters

(Happens on 7.4.26 too, from Wamp 3.2.6)

Voltra

Voltra commented on Mar 10, 2023

@Voltra

In the meantime, here's the rector config I use to circumvent the issue:

// rector.php
$rectorConfig->paths([
    __DIR__ . '/src',
    __DIR__ . '/vendor/symfony/',
    __DIR__ . '/vendor/doctrine/',
   // Add any dependency that causes issues, adding the whole vendor directory might be too much on your PC
]);

$rectorConfig->ruleWithConfiguration(RenameFunctionRector::class, [
    'rename' => 'windows_rename_fix',
]);
// monkey_patch.php
function windows_rename_fix(string $oldName, string $newName, $context = null): bool {	
	$didCopy = copy($oldName, $newName, $context);

	if (!$didCopy) {
		return false;
	}

	@unlink($oldName, $context);

	return true;
}

if (!function_exists('rename')) {
	function rename(string $oldName, string $newName, $context = null): bool {
		return windows_rename_fix($oldName, $newName, $context);
	}
}

And I add the monkey_patch.php in the autoload.files array in my composer.json

lufog

lufog commented on Mar 11, 2024

@lufog

Does this relate to the fact that in PHP 8 (8.1.27, 8.2.16, 8.3.3), on Windows, unlink() cannot delete a currently running script? Since PHP 7 (7.4.33) doesn't have this problem, I guess it's a regression?

nicolas-grekas

nicolas-grekas commented on Jul 5, 2024

@nicolas-grekas
Contributor

Is this issue still relevant? Didn't @cmb69 fix it in c7f24d9?

nicolas-grekas

nicolas-grekas commented on Jul 5, 2024

@nicolas-grekas
Contributor

Ah, yes, #9351 is still opened as draft. Would be could is someone could take over!

Voltra

Voltra commented on Jul 5, 2024

@Voltra

I'm not sure if I could reproduce it as it was very "random" as to when the problem would occur, and I don't currently have access to the repo that had the issue

gggeek

gggeek commented on Jul 26, 2024

@gggeek

@nicolas-grekas I dont have MS build tooling installed any more, but I can help testing #9351 as long as there are prebuilt binaries I can download...

lufog

lufog commented on Jul 26, 2024

@lufog

I'm not sure if I could reproduce it as it was very "random" as to when the problem would occur, and I don't currently have access to the repo that had the issue

The problem is reproducible in 100% of cases when updating GravCMS on Windows with PHP 8+ (on PHP 7 everything is fine).

Voltra

Voltra commented on Jul 26, 2024

@Voltra

That's interesting as my use case was Symfony's entity classes (or something related) cache and getting to hit the bug mid-request (i.e. when the file would be open/read prior to being replaced) was complex

johnstevenson

johnstevenson commented on Jul 27, 2024

@johnstevenson
ContributorAuthor

I'm not sure if I could reproduce it ...

See #7910 (comment)

<?php
// file: bug.php

$content = file_get_contents(__FILE__);
file_put_contents('newfile.php', $content."\n//");
var_dump(rename('newfile.php', __FILE__));
Voltra

Voltra commented on Aug 14, 2024

@Voltra

Using my current WAMP setup, here are the PHP versions that suffer from the bug:

  • 8.1.0
  • 8.1.26
  • 8.1.27
  • 8.1.28
  • 8.1.29
  • 8.2.18
  • 8.3.6

Here are the versions that don't:

  • 7.1.33
  • 7.4.26
  • 8.0.13
removed their assignment
on Sep 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Participants

      @nicolas-grekas@gggeek@johnstevenson@cmb69@lufog

      Issue actions

        rename fails on Windows PHP 8.1 if the target file is being executed · Issue #7910 · php/php-src