Problem/Motivation

When calling TemporaryStream::getExternalUrl() I get this error:

Fatal error: Call to undefined method Drupal\Core\StreamWrapper\TemporaryStream::url() in /var/www/drupal8/core/lib/Drupal/Core/StreamWrapper/TemporaryStream.php on line 33

Proposed resolution

Fix this by calling Url::fromRoute() instead of non-existent function.

Remaining tasks

- prepare patch
- write tests
- review

User interface changes

None

API changes

None

CommentFileSizeAuthor
#11 interdiff.txt1.06 KBslashrsm
#11 2352855_11.patch2.94 KBslashrsm
#7 interdiff.txt1.36 KBslashrsm
#7 2352855_7.patch2.29 KBslashrsm
#1 2352855_1_TEST_ONLY.patch1.25 KBslashrsm
#1 2352855_1.patch2.04 KBslashrsm

Comments

slashrsm’s picture

slashrsm commented
Status: Active » Needs review
StatusFileSize
new 2352855_1.patch2.04 KB
new 2352855_1_TEST_ONLY.patch1.25 KB
slashrsm’s picture

slashrsm commented
Issue summary: View changes

Status: Needs review » Needs work

The last submitted patch, 1: 2352855_1_TEST_ONLY.patch, failed testing.

slashrsm’s picture

slashrsm commented
Status: Needs work » Needs review
primsi’s picture

primsi commented
Status: Needs review » Reviewed & tested by the community

Looks ok.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/system/src/Tests/File/StreamWrapperTest.php
@@ -84,6 +84,15 @@ function testUriFunctions() {
+    $this->assertTrue(file_create_url('temporary://test.txt'), t('Temporary external URL correctly built.'));
+    $this->assertTrue(file_create_url('public://test.txt'), t('Public external URL correctly built.'));
+    $this->assertTrue(file_create_url('private://test.txt'), t('Private external URL correctly built.'));

Not supposed to use t() on test assertion messages and we're not really testing the that the URL is correct here by user assertTrue are we?

slashrsm’s picture

slashrsm commented
Status: Needs work » Needs review
StatusFileSize
new 2352855_7.patch2.29 KB
new interdiff.txt1.36 KB

Ah, right! Here we go.

Ibn al-Hazardous’s picture

Ibn al-Hazardous commented

Hello,
is this patch going into the next beta?

I get a white screen of death going to /admin/content/files, and the log shows the same fatal error as in the initial report.

The latest patch works perfectly for me, so who do I pester to see it applied? ;-)

slashrsm queued 7: 2352855_7.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 7: 2352855_7.patch, failed testing.

slashrsm’s picture

slashrsm commented
Status: Needs work » Needs review
StatusFileSize
new 2352855_11.patch2.94 KB
new interdiff.txt1.06 KB
marcingy’s picture

marcingy commented
Status: Needs review » Reviewed & tested by the community

Looks good

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 11: 2352855_11.patch, failed testing.

slashrsm queued 11: 2352855_11.patch for re-testing.

slashrsm’s picture

slashrsm commented
Status: Needs work » Reviewed & tested by the community
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.0.x, thanks!

  • catch committed d3703f9 on 8.0.x 
    Issue #2352855 by slashrsm: TemporaryStream::getExternalUrl() is broken

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.