Problem/Motivation

Drupal may be configured to allow untrusted users to upload files with arbitrary filenames, including filenames with path components. For instance, if Drupal is installed at /var/www/ and the uploaded file repository is at /var/www/sites/example.com/files/, then an uploaded filename of '../../../index.php' would point to the /var/www/index.php file.

To prevent untrusted users from overwriting such critical files, the current logic for the DrupalLocalStreamWrapper::getLocalPath() function works like this:

  1. Set $directory to the realpath() of the files repository.

  2. Set $path to the target file location.

  3. Set $realpath to realpath($path).

  4. Return FALSE if $realpath does not begin with $directory.

This causes a problem when the site builder wants to store some uploaded files outside of the Drupal web root. Many site owners have resorted to hard-linked directories, recursive bind mounts, or simply hacking Drupal core to remove the restriction.

Proposed resolution

The logic within the DrupalLocalStreamWrapper::getLocalPath() function should be revised as follows:

  1. Set $directory to the realpath() of the files repository.

  2. Set $path to the target file location.

  3. Set $realpath to realpath($path).

  4. Return FALSE if both of the following are true:

    • $path contains '/..' or '\..'.
    • $realpath does not begin with $directory

    .

    Remaining tasks

    The patch proposed in #65 needs to be reviewed by the security team.

    User interface changes

    None.

    API changes

    The DrupalLocalStreamWrapper::getLocalPath() function would follow symlinks, even if they point outside the files repository.

    Original report by tekante

    Related issue: #155781: "realpath" check breaks symbolic links in file directory

    The method by which directory ascension attacks are prevented in stream_wrappers.inc prevents file system layouts which rely on symlinks within the files directory. Specifically, the use of realpath within the getLocalPath results in the check of whether the destination directory exists within the files directory to fail.

    I will attach a patch which allows for the destination directory to exist outside of the configured files directory when it appears this is due to the presence of a symlink and not due to directory ascension characters.

    Files: 
    CommentFileSizeAuthor
    #100 stream_wrappers-allow_symlinks-1008402-99.patch3.94 KBbdone
    PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,527 pass(es).
    [ View ]

    Comments

    tekante’s picture

    StatusFileSize
    new1.55 KB
    PASSED: [[SimpleTest]]: [MySQL] 28,980 pass(es).
    [ View ]

    Patch to allow use of symlinks within files directory attached

    febbraro’s picture

    Priority:Normal» Critical
    Status:Active» Needs review

    Moving to "Needs Review" for the test bot, and bumping to critical b/c this breaks symlink usage from D6 & prior.

    carlos8f’s picture

    Priority:Critical» Major

    I think using symlinks in your files directory is edge-case and qualifies more as 'major'.

    Priority levels of issues

    febbraro’s picture

    Version:7.0-rc1» 7.0-rc3

    I was under the impression that if it was a regression of something that worked in the previous release (D6), then it was considered critical. I do not see that scenario mentioned anywhere in that Priority document. Who would we ask to provide some clarification on that?

    [I do not consider using symlinks for the files directory edge-case, we host well over 500 drupal sites and they all use symlinks for the files directory as do many other companies that I know]

    carlos8f’s picture

    To put it in perspective, PostgreSQL issues are not considered critical... we only consider an issue critical if it causes data loss, security holes, or stops multiple pieces of functionality from working for most people. Symlinks aren't even a feature of Drupal per se, so technically it is not really a regression. I still think this is a valuable issue to address, but not critical by the community's standards.

    moshe weitzman’s picture

    Not sure if it is related, but here is an older, similar issue #155781: "realpath" check breaks symbolic links in file directory which is resolved for d7 core but not for imagecache project.

    febbraro’s picture

    @moshe thank for the pointer. @carlos8f, thanks for the clarification, makes sense.

    bfroehle’s picture

    Just marked #1055170: Drupal public and private file schemes are not supported junctions (symlinks) of Windows NTFS! as a duplicate. Any change here will need to be cognizant of "junctions" --- what symlinks are called in the land of NTFS.

    W32’s picture

    The Cause of bug on NTFS junction is described in http://drupal.org/node/1055170#comment-4074362 (closed duplicate #1055170: Drupal public and private file schemes are not supported junctions (symlinks) of Windows NTFS! ).
    I propose to remove stpos from DrupalLocalStreamWrapper::getLocalPath() but, as bfroehle specify strpos is a security check. In this case, we need to implement own analogue of realpath() for call in getLocalPath(). But this analogue should not transform symlink (junction) to real path. In this case, method getLocalPath() will always work with path that is relative to scheme base folder.

    W32’s picture

    Version:7.0-rc3» 7.x-dev

    I change issue version to 7.x-dev, because this bug exists in 7.0 release and in current development brunch too.

    bfroehle’s picture

    Status:Needs review» Needs work
    --- 367,382 ----
    +       $symlink = ($realpath != $dirpath && !preg_match('/\.\./', $dirpath) && !preg_match('/~/', $dirpath));

    The logic here is hard to follow. Since we would have to be very careful here not to introduce a security vulnerability, can you add some descriptive comments about what exactly is going on?

    Powered by Dreditor.

    tekante’s picture

    Status:Needs work» Needs review
    StatusFileSize
    new2.13 KB
    PASSED: [[SimpleTest]]: [MySQL] 31,609 pass(es).
    [ View ]

    Attached patch with comments explaining use of the symlink flag and what the regular expressions are checking for

    sun’s picture

    Version:7.x-dev» 8.x-dev
    Status:Needs review» Needs work
    Issue tags:+needs backport to D7
    --- 367,389 ----
    +       // as a security mechanism $directory values which don't appear in the $realpath
    +       // value result in a FALSE return to prevent directory ascension attacks
    +       // this breaks systems which want to symlink parts of the filesystem to somewhere
    +       // outside of $realpath. This sets the $symlink flag if it appears this has happened
    +       // only if no directory ascenension attack strings are observed. Flag is later used to ¶
    +       // allow the $directory values which don't appear in $realpath
    ...
    +       // see comment in true case for details about symlink flag test

    1) We need a patch created using git against latest code in 8.x.

    2) The patch needs to be in unified diff format.

    3) Please read http://drupal.org/node/1354 for Drupal's coding standards for comments.

    Powered by Dreditor.

    pillarsdotnet’s picture

    StatusFileSize
    new1.73 KB
    FAILED: [[SimpleTest]]: [MySQL] 29,404 pass(es), 262 fail(s), and 11,496 exception(es).
    [ View ]

    Improved logic and comments. Could be better. Is there a function that returns the directory path separator character? (usually '\' for Windows and '/' for Unix).

    EDIT: Found it. I wanted the DIRECTORY_SEPARATOR constant.

    Title:Can not use symlinks within the files directory» Allow the use of symlinks within the files directory.

    The last submitted patch, stream_wrappers-allow_symlinks-1008402-14.patch, failed testing.

    pillarsdotnet’s picture

    Status:Needs work» Needs review
    StatusFileSize
    new1.73 KB
    FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in includes/stream_wrappers.inc.
    [ View ]

    Status:Needs review» Needs work

    The last submitted patch, stream_wrappers-allow_symlinks-1008402-16.patch, failed testing.

    sun’s picture

    1) It doesn't look like #16 has been tested locally in any way. The syntax error is obvious compared to #14.

    2) The code could explain the reasoning for what is being done a bit better. The comments from the old patch might be a starting point, though also not sufficiently clear.

    3) The if condition should not wrap. Since you keep on doing this, I'll make sure to formalize the rules in Drupal's coding standards.

    pillarsdotnet’s picture

    Status:Needs work» Needs review
    StatusFileSize
    new2.1 KB
    FAILED: [[SimpleTest]]: [MySQL] 29,421 pass(es), 259 fail(s), and 11,468 exception(es).
    [ View ]

    @3) -- Please do. I'll keep breaking unwritten rules until they get written down.

    @1-2) -- Better? EDIT: Bah; now I see; the backslashes need to be quadrupled. Rebuilding and locally testing now.

    The last submitted patch, stream_wrappers-allow_symlinks-1008402-19.patch, failed testing.

    pillarsdotnet’s picture

    StatusFileSize
    new2.2 KB
    PASSED: [[SimpleTest]]: [MySQL] 29,994 pass(es).
    [ View ]

    Added additional comments which hopefully explains what is going on. If it is still unclear, please let me know and I'll try to be more verbose.

    jbrown’s picture

    subscribing

    pillarsdotnet’s picture

    StatusFileSize
    new3.97 KB
    PASSED: [[SimpleTest]]: [MySQL] 31,434 pass(es).
    [ View ]

    Improved comments.

    Used the DIRECTORY_SEPARATOR constant to replace the confusing '(/|\\\\)' pattern.

    bfroehle’s picture

    Bob, I'm not sure about using the constant... The issue is that on windows both / and \ are accepted directory separators, even though \ is preferred. (note, however, that mixing of / and \ in the same path is not allowed)

    pillarsdotnet’s picture

    StatusFileSize
    new3.94 KB
    PASSED: [[SimpleTest]]: [MySQL] 31,442 pass(es).
    [ View ]

    note, however, that mixing of / and \ in the same path is not allowed

    Actually, the Windows command line does allow mixing them in the same path; I've done it. However, realpath() emits backslashes on windows and forward-slashes on unix, so using the constant is justified for the strpos() portion of the test.

    Reverted back to the pattern for the preg_match() portion. Pity it looks so screwball with the quadrupled backslash characters.

    pillarsdotnet’s picture

    StatusFileSize
    new5.13 KB
    PASSED: [[SimpleTest]]: [MySQL] 33,627 pass(es).
    [ View ]
    new2.62 KB
    FAILED: [[SimpleTest]]: [MySQL] 31,445 pass(es), 1 fail(s), and 0 exception(es).
    [ View ]

    Removed leftover cruft from local testing and added an explicit test for symlink support.

    xjm’s picture

    Tagging issues not yet using summary template.

    pillarsdotnet’s picture

    bradhawkins’s picture

    Subscribe

    catch’s picture

    Issue tags:+needs security review

    Adding 'needs security review' tag.

    bvanmeurs’s picture

    Subscribe. This is an important issue for me. I don't see why the check 'strpos($realpath, $directory) !== 0' is needed at all in DrupalLocalStreamWrapper::getLocalPath. Can someone explain why this check is needed?

    Right now, I'm using hardlinks instead of symbolic links.

    pillarsdotnet’s picture

    Updated summary.

    carlos8f’s picture

    xjm’s picture

    Status:Needs review» Needs work
    Issue tags:+Novice

    Note that the Drupal 8.x patch will need to be rerolled, because the core directory structure for Drupal 8 has now changed. (For more information, see #22336: Move all core Drupal files under a /core folder to improve usability and upgrades). When the patch has been rerolled, please set the issue back to "Needs Review."

    Tagging as novice for the task of rerolling the Drupal 8.x patch.

    If you need help rerolling this patch, you can come to core office hours or ask in #drupal-gitsupport on IRC.

    kathyh’s picture

    StatusFileSize
    new4.24 KB
    PASSED: [[SimpleTest]]: [MySQL] 33,812 pass(es).
    [ View ]

    #34 Re-roll for D8 /core change

    kathyh’s picture

    Status:Needs work» Needs review
    xjm’s picture

    Issue tags:-Novice

    Thanks!

    joelcollinsdc’s picture

    subscribe

    xjm’s picture

    Stop subscribing; start following

    Also, the issue is set to Needs Review, which means it could use testing to verify whether the patch resolves the issue. :)

    xjm’s picture

    StatusFileSize
    new4.2 KB
    FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch stream_wrappers-allow_symlinks-1008402-40-d7.patch. Unable to apply patch. See the log in the details link for more information.
    [ View ]

    D7 version of the patch.

    KingSalibah’s picture

    @xjm Would you mind taking a look at http://drupal.org/node/1347222? I believe I am having a similar, if not the same, issue. I would like to know if this patch (for D7) would work for my issue. I was told D7 does not support symlinks, however, it did in essence work, and if it could work without showing me an error, it would be a great solution for my site. Thanks.

    xjm’s picture

    #41: The best thing to do would be to apply the patch in #40 and actually test whether it resolves the issue. Then, report your findings in this issue. That's what's needed at this time in order for this to be fixed.

    KingSalibah’s picture

    Okay, I am trying my first patch!

    First error, can't find the filename. Why does the path have "a/" and "b/"?

    I took those out and running:

    patch -p0 --dry-run <stream_wrappers-allow_symlinks-1008402-40-d7.patch, I got the following result:

    patching file includes/stream_wrappers.inc
    Hunk #1 succeeded at 365 (offset -8 lines).
    patching file modules/simpletest/tests/file.test

    Thus, seems like it would work without a/ and b/. I dunno what "at 365 (offset -8 lines)" means though.

    jbrown’s picture

    @KingSalibah read http://drupal.org/node/707484 to learn how to apply a patch.

    xjm’s picture

    #43: Yeah, the a/ and b/ are there for git patches. You can either use patch -p1 to apply them, or git apply. The message about offset is okay, so it looks like it was correctly applied. Glad to see you got that working. :) Now, does it fix the symlinks issue for your site?

    KingSalibah’s picture

    Patch worked successfully on my Drupal 7.8 site.

    [I will spare you my newbie travails, but make no mistake, a designer/developer of a website is not the same as a server administrator or linux user. The broad knowledge required to be a real Drupal admin is quite extensive. There appears to be no light-hearted versions of Drupal admin. You are either in or you're out, unless of course all you want is a cheesy blog.]

    I installed git and patched it that way. Results were clean and seems to work fine so far.

    xjm’s picture

    Thanks @KingSalibah!

    xjm’s picture

    Issue summary:View changes

    Updated to use Issue Summary Template.

    chx’s picture

    So a symlink pointing to /etc/passwd is good..?

    xjm’s picture

    So a symlink pointing to /etc/passwd is good..?

    Well, it is tagged "needs security review"...

    donquixote’s picture

    So a symlink pointing to /etc/passwd is good..?

    Good question.
    First, one could ask, how did the symlink get there in the first place?
    OK, probably because www-data has write access on these directories.
    Something hacked the site, and lets it create symlinks to evil locations.
    Or, it could even be another site that was hacked, one that has the same www-data user.

    What about a whitelist in settings.php, for known symlink destinations?

    ufku’s picture

    Status:Needs review» Needs work

    Tilde(~) is interpreted as is by PHP. It's the shell that interprets it as home dir. It also works only when it is at the beginning like ~/. So the tilde part of the patch is overhead.

    pillarsdotnet’s picture

    @#48-#50

    So a symlink pointing to /etc/passwd is good..?

    I don't think it is possible to create a symlink using the Drupal API. So if there is a security problem, it isn't with Drupal. Finally, if the www-data user has write-access to /etc/password, there is definitely a security problem outside of Drupal. This is especially true of a host with multiple sites running as the same www-data user.

    @#51

    Tilde(~) is interpreted ... by ... the shell ... the tilde part of the patch is overhead.

    Can we guarantee that the shell will never get involved? What about Hostmonster, where PHP is invoked via CGI?

    ufku’s picture

    Can we guarantee that the shell will never get involved? What about Hostmonster, where PHP is invoked via CGI?

    Why would we have to guarantee that?
    If the shell is involved, in the first place, we cannot guarantee anything.

    pillarsdotnet’s picture

    Status:Needs work» Needs review
    StatusFileSize
    new4.78 KB
    FAILED: [[SimpleTest]]: [MySQL] 33,325 pass(es), 7 fail(s), and 0 exception(es).
    [ View ]

    #35 re-rolled with the tilde(~) portions removed and minor comment improvements. Still needs security review.

    Status:Needs review» Needs work

    The last submitted patch, stream_wrappers-allow_symlinks-1008402-54.patch, failed testing.

    pillarsdotnet’s picture

    Status:Needs work» Needs review
    StatusFileSize
    new6.34 KB
    FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch stream_wrappers-allow_symlinks-1008402-56.patch. Unable to apply patch. See the log in the details link for more information.
    [ View ]

    Corrected and re-rolled. One of the file tests should be skipped if clean urls are disabled.

    pillarsdotnet’s picture

    Issue summary:View changes

    Updated issue summary.

    pillarsdotnet’s picture

    Issue summary:View changes

    Removed mention of tilde (~) as per ufku because the tilde is interpreted by the shell, not by PHP. Drupal is not responsible for protecting against shell-based exploits.

    DamienMcKenna’s picture

    The patch in #40 has solved a problem on a server where the images were being loaded via a mount point from a SAN (75gb of data that was being references via custom entities), so a huge HighFive from me.

    pillarsdotnet’s picture

    Status:Needs review» Needs work
    Issue tags:+needs security review, +needs backport to D7

    The last submitted patch, stream_wrappers-allow_symlinks-1008402-56.patch, failed testing.

    pillarsdotnet’s picture

    Status:Needs work» Needs review
    StatusFileSize
    new6.39 KB
    PASSED: [[SimpleTest]]: [MySQL] 35,622 pass(es).
    [ View ]

    Re-rolled.

    tomogden’s picture

    Thanks for all your really hard work. You have saved me and others a great deal of time putting all of this together so nicely.

    I'm not sure what branch you generated the patch from, but as alluded to in #43, the core/modules/simpletest/tests directory and the core/modules/simpletest/tests/file.test file don't appear in the D8.x install.

    I wonder if you wouldn't mind clearing up that point, and I would love to run some tests on it.

    tomogden’s picture

    Okay, I see what happened. All the testing files were moved out of the SimpleTest module, so we have to reroll the patch to reflect that update.

    See #1352000: Forward-port upgrade test clean-ups from 7.x to 8.x.

    tomogden’s picture

    Status:Needs review» Needs work

    In reviewing your patch, I also found a conflict with #1183208: Remove variable_get('clean_url') and switch to index.php/path pattern for dirty URL support. Just a minor point where you are inserting if (variable_get('clean_url', 0)). We need to replace that so as to not undo their work.

    tomogden’s picture

    Assigned:Unassigned» tomogden
    Status:Needs work» Needs review
    StatusFileSize
    new4.93 KB
    PASSED: [[SimpleTest]]: [MySQL] 36,499 pass(es).
    [ View ]

    Here's the patch, updated to accommodate the above issues.

    pillarsdotnet’s picture

    StatusFileSize
    new4.85 KB
    PASSED: [[SimpleTest]]: [MySQL] 36,515 pass(es).
    [ View ]

    Minor correction to a grammar mistake that I originally introduced in #60.

    pillarsdotnet’s picture

    Issue summary:View changes

    Updated links to current patch.

    tomogden’s picture

    Status:Needs review» Reviewed & tested by the community

    Great so I have pulled the latest D8, and this patch meets my tests. All the security checks I know are in line. Let's call it 'reviewed and tested.'

    pillarsdotnet’s picture

    Status:Reviewed & tested by the community» Needs review

    can't rtbc your own patch.

    andypost’s picture

    Patch looks good but we need more reviews to check that contrib modules a-la cdn still works.
    PS: and it still needs security review

    pillarsdotnet’s picture

    Assigned:tomogden» Unassigned

    Unassigning; needs review by security team.

    batje’s picture

    StatusFileSize
    new4.2 KB
    FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch stream_wrappers-allow_symlinks-1008402-70-D7.patch. Unable to apply patch. See the log in the details link for more information.
    [ View ]

    this is #65 for D7

    Status:Needs review» Needs work

    The last submitted patch, stream_wrappers-allow_symlinks-1008402-70-D7.patch, failed testing.

    mukhsim’s picture

    Patch in #1 no longer applies to D-7.14. The attached patch is a stopgap measure to have symlink functionality available in current Drupal releases while a proper backport is worked out.

    Mukhsim.

    pillarsdotnet’s picture

    Status:Needs work» Needs review

    #65 still needs review by security team.

    Leeteq’s picture

    I see it has been tagged and awaiting security team review for 2+ months.
    Would be great if this makes it in in time to be part of 7.15.

    tim.plunkett’s picture

    Category:bug» feature
    Issue tags:+Needs issue summary update

    The title and OP imply that this is a feature.
    If it is truly a bug, can the issue summary be updated to explain it?

    pillarsdotnet’s picture

    Untagging. The issue summary is accurate. This is indeed a feature request, despite the fact that the requested feature existed in previous Drupal versions and was removed for security reasons.

    Still needs review by security team before moving forward. So noted in summary, tags, and in several comments. And just for the sight-impaired:

    NEEDS REVIEW BY SECURITY TEAM

    ianthomas_uk’s picture

    OK, so this needs review by the security team, but that's been the case for coming up to two years. Is there anything someone who is not on the security team can do to help? Looking at the security team pages they seem more focused on fixing known vulnerabilities rather than reviewing new code.

    RunePhilosof’s picture

    I have tested #70 in D7.22 and it somewhat works. But for some reason is_link("public://link") returns false when it should not.

    RunePhilosof’s picture

    If you have a structure as such:
    link -> dir
    dir/

    A streamwrapper should remove the symlink if ->unlink('link') is called, it should not attempt to remove the directory.
    And if ->rmdir('link') is called it should also fail, because it is not a directory.

    I see that always dereferencing symlinks in the streamwrapper is the fastest way to make drupal support symlinks.
    However, I would prefer adhering to the streamwrapper interface and fix the other places in core that would fail because of this.

    RunePhilosof’s picture

    Status:Needs review» Needs work
    RunePhilosof’s picture

    StatusFileSize
    new2.83 KB
    FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch interdiff.64-65.diff. Unable to apply patch. See the log in the details link for more information.
    [ View ]

    In #65 you said that you only corrected a minor typo.
    But I see that you also deleted a change in file.test.
    Was that intentional?

    andypost’s picture

    Status:Needs work» Needs review

    let's see

    andypost’s picture

    Status:Needs review» Needs work
    Issue tags:+Needs reroll
    ekl1773’s picture

    Assigned:Unassigned» ekl1773

    Assigning reroll to self for Core Mentoring ...

    RunePhilosof’s picture

    I've been thinking that maybe it isn't so bad that symlinks are made transparent by always dereferencing.

    However, this should be fixed:
    link -> dir
    is_dir("public://link/..") #returns FALSE.

    RunePhilosof’s picture

    In order to properly test this functionality all drupals tests should be run with the files directory symlinked.
    That would probably catch a lot of edge cases.

    ekl1773’s picture

    Assigned:ekl1773» Unassigned
    Status:Needs work» Needs review
    Issue tags:-Needs reroll
    StatusFileSize
    new4.19 KB
    FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/lib/Drupal/system/Tests/File/DirectoryTest.php.
    [ View ]

    Rerolled patch from #64, but it still needs a quick typo fix from #65. See also #81.

    Status:Needs review» Needs work

    The last submitted patch, stream_wrappers-allow_symlinks-1008402-87.patch, failed testing.

    mradcliffe’s picture

    +++ b/core/modules/system/lib/Drupal/system/Tests/File/DirectoryTest.phpundefined
    @@ -162,3 +162,40 @@ function testFileDirectoryTemp() {
         $this->assertEqual($config->get('path.temporary'), $tmp_directory);
       }
    }
    +
    +  /**
    +   * Tests that symlinks are supported within the files directory.
    +   */

    Note that there are 2 ending curly braces. The last one doesn't have any space indentation so it must mean the end of the test class. And then immediately a function is defined outside of that, testFileDirectorySymLinks(). The error is that this function is a method within the PHP class - the second ending curly brace here should be removed.

    +++ b/core/modules/system/lib/Drupal/system/Tests/File/DirectoryTest.phpundefined
    @@ -162,3 +162,40 @@ function testFileDirectoryTemp() {
    +    // Clean up the mess.
    +    file_unmanaged_delete($destination);
    +    drupal_unlink($symlink);
    +    drupal_rmdir($temp_dir);
    +  }
    +}

    At the end of testFileDirectorySymLinks() there are two ending curly braces.

    This is correct despite the syntax error suggesting its this line.

    ekl1773’s picture

    Status:Needs work» Needs review
    StatusFileSize
    new4.29 KB
    FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch stream_wrappers-allow_symlinks-1008402-90.patch. Unable to apply patch. See the log in the details link for more information.
    [ View ]

    Corrected PHP syntax error as above and corrected the typo that was originally corrected in #65. This should be ready for review.

    RunePhilosof’s picture

    For what purpose do we allow .. in the paths.
    Could it be disallowed entirely?

    If .. is being used by the system, then it would fail if you make some path into a symlink.

    RunePhilosof’s picture

    Rerolled the D7 version from #70.

    RunePhilosof’s picture

    Issue summary:View changes

    Updated link to patch under review.

    xjm’s picture

    Status:Needs review» Needs work

    The last submitted patch, 90: stream_wrappers-allow_symlinks-1008402-90.patch, failed testing.

    ciss’s picture

    Issue summary:View changes

    Rerolled patch from #92 applies smoothly to 7.29.

    geerlingguy’s picture

    ciss, would you mind posting your rerolled patch? :)

    ciss’s picture

    @geerlingguy I didn't reroll any patches. I simply used the patch from comment #92 that had been rerolled by RunePhilosof.
    If "patch <" shouldn't work for you (I didn't test it), try "git apply" which worked for me on a git checkout of the 7.29 revision.

    bdone’s picture

    Status:Needs work» Needs review
    StatusFileSize
    new4.23 KB
    FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,170 pass(es), 2 fail(s), and 0 exception(s).
    [ View ]

    here's a re-roll of #90, that corrects the test's path:
    from: /core/modules/system/lib/Drupal/system/Tests/File/DirectoryTest.php
    to: /core/modules/system/src/Tests/File/DirectoryTest.php

    Status:Needs review» Needs work

    The last submitted patch, 98: stream_wrappers-allow_symlinks-1008402-98.patch, failed testing.

    bdone’s picture

    Status:Needs work» Needs review
    StatusFileSize
    new3.94 KB
    PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,527 pass(es).
    [ View ]
    new2.23 KB

    here's an updated version of #98, but re-rolled for HEAD, and removing the variable_set/gets all together in the test. this also changes the test file to use a static file, since as it is, existing test didn't have access to WebTestBase::drupalGetTestFiles.

    chx’s picture

    Noone answered #48 yet in the last three years.

    andypost’s picture

    @chx #48 is not a question. The reply in #50 and #52, who and how it could be possible...
    and actually how it's related to patch that simply allows to store files not only within "files" folder?

    agerson’s picture

    #92 worked for me on D7. We keep our files on an external storage so this is very helpful!

    Bobík’s picture

    #92 works for D7.

    How about to commit it or make separated issue for D7 to speedup fixing for D7?