Skip to content

Conversation

joecorall
Copy link
Member

@joecorall joecorall commented Jul 3, 2025

What does this Pull Request do?

Fixes a bug when downloading files from fedora that have a comma in the URI.

Consider a drupal flysystem URI like

/_flysystem/fedora/2024-12/Rezak%2C%20Craze%2C%20Milton%2C%20and%20Poncy_complete.mov

When Drupal requests the file from fedora, fedora returns headers like this (notice %2C percent encoding no longer exists ,)

< Link: <http://fcrepo:8080/fcrepo/rest/2024-12/Rezak,%20Craze,%20Milton,%20and%20Poncy_complete.mov/fcr:metadata>; rel="describedby"
< Link: <http://fcrepo:8080/fcrepo/rest/2024-12/Rezak,%20Craze,%20Milton,%20and%20Poncy_complete.mov>; rel="timegate"
< Link: <http://fcrepo:8080/fcrepo/rest/2024-12/Rezak,%20Craze,%20Milton,%20and%20Poncy_complete.mov>; rel="original"
< Link: <http://fcrepo:8080/fcrepo/rest/2024-12/Rezak,%20Craze,%20Milton,%20and%20Poncy_complete.mov/fcr:versions>; rel="timemap"
< Link: <http://www.w3.org/ns/ldp#NonRDFSource>; rel="type"
< Link: <http://www.w3.org/ns/ldp#Resource>; rel="type"
< Link: <http://fedora.info/definitions/v4/repository#Resource>; rel="type"
< Link: <http://mementoweb.org/ns#OriginalResource>; rel="type"
< Link: <http://mementoweb.org/ns#TimeGate>; rel="type"
< Link: <http://fedora.info/definitions/v4/repository#Binary>; rel="type"
< Link: <http://fcrepo:8080/fcrepo/rest/2024-12/Rezak,%20Craze,%20Milton,%20and%20Poncy_complete.mov/fcr:acl>; rel="acl"

And subsequently GuzzleHttp\Psr7\Header::parse will split on the commas, returning headers in an unexpected format.

The way flysystem handles any errors (even warnings) causes the request to fail (from watchdog)

RuntimeException: Undefined array key "rel" in trigger_error() (line 938 of /var/www/drupal/vendor/codementality/flysystem-stream-wrapper/src/FlysystemStreamWrapper.php)
#0 /var/www/drupal/vendor/codementality/flysystem-stream-wrapper/src/FlysystemStreamWrapper.php(938): trigger_error()
#1 /var/www/drupal/vendor/codementality/flysystem-stream-wrapper/src/FlysystemStreamWrapper.php(905): Codementality\FlysystemStreamWrapper\FlysystemStreamWrapper->triggerError()
#2 /var/www/drupal/vendor/codementality/flysystem-stream-wrapper/src/FlysystemStreamWrapper.php(518): Codementality\FlysystemStreamWrapper\FlysystemStreamWrapper->invoke()
#3 [internal function]: Codementality\FlysystemStreamWrapper\FlysystemStreamWrapper->stream_open()
#4 /var/www/drupal/vendor/symfony/http-foundation/BinaryFileResponse.php(321): SplFileObject->__construct()
#5 /var/www/drupal/vendor/symfony/http-foundation/Response.php(395): Symfony\Component\HttpFoundation\BinaryFileResponse->sendContent()
#6 /var/www/drupal/web/index.php(20): Symfony\Component\HttpFoundation\Response->send()
#7 {main}

How should this be tested?

I feel this change is straightforward enough it does not need tested. Though i guess uploading a file that will be percent encoded in fedora, then trying to download it, will make the error trip. This test script also reveals the error

<?php

use GuzzleHttp\Psr7;

$request = new Psr7\Request('GET', '/', [
    'Link' => '<http:/.../Rezak%2C%20Craze,%20Milton%2C%20and%20Poncy_complete.mov>; rel="front"; type="image/jpeg"'
]);

$parsed = Psr7\Header::parse($request->getHeader('Link'));
var_export($parsed);

Then run the script and you can see the first element of the array does not have the rel key

$ drush scr test.php
array (
  0 =>
  array (
    0 => '<http:/.../Rezak%2C%20Craze',
  ),
  1 =>
  array (
    0 => '%20Milton%2C%20and%20Poncy_complete.mov>',
    'rel' => 'front',
    'type' => 'image/jpeg',
  ),

Interested parties

@Islandora/committers
@whikloj

@joecorall joecorall changed the title Fix fedora download bug Fix fedora download bug for files with commas in the filename Jul 3, 2025
@seth-shaw-asu seth-shaw-asu self-requested a review July 23, 2025 17:21
@seth-shaw-asu seth-shaw-asu merged commit d6c8ce9 into 2.x Aug 13, 2025
24 checks passed
@seth-shaw-asu seth-shaw-asu deleted the bugfix-rel branch August 13, 2025 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants