The hook_file_transfer function in xmlsitemap module (line 2223) expects parameter one to be a Response object.
It seems that the module file_entity invokes this hook and it passes following arguments:

 \Drupal::moduleHandler()->invokeAll('file_transfer', array($file->getFileUri(), $headers));

So this generates a fatal error... I looked into the code of xmlsitemap_file_transfer and all it does is adding header to the response object... what is the use of this function, I don't think this is needed. If so it has to be done in "hook_file_download_headers_alter" though.

CommentFileSizeAuthor
#6 xmlsitemap_hook_file_transfer-2831733-6.patch871 bytesCyberwolf
#3 hook_file_transfer-2831733-3.patch1.47 KBrobin.ingelbrecht
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

robin.ingelbrecht created an issue. See original summary.

robin.ingelbrecht’s picture

robin.ingelbrecht CreditAttribution: robin.ingelbrecht commented
Issue summary: View changes
robin.ingelbrecht’s picture

robin.ingelbrecht CreditAttribution: robin.ingelbrecht commented
FileSize
hook_file_transfer-2831733-3.patch1.47 KB

Attached patch removes this function as I think it is obsolete...

TheodorosPloumis’s picture

TheodorosPloumis
he/him
Location Greece
CreditAttribution: TheodorosPloumis commented

I came to this issue because function xmlsitemap_file_transfer caused fatal errors when trying to download files form the file_entity module widget. It was exactly the same problem.

The patch solved the issue since it remove completely the xmlsitemap_file_transfer function.

TheodorosPloumis’s picture

TheodorosPloumis
he/him
Location Greece
CreditAttribution: TheodorosPloumis commented
Title: hook_file_transfer » xmlsitemap_file_transfer causes fatal errors
Issue tags: +http_response
Cyberwolf’s picture

Cyberwolf CreditAttribution: Cyberwolf at 2DotsTwice bvba commented
FileSize
xmlsitemap_hook_file_transfer-2831733-6.patch871 bytes

xmlsitemap_output_file() still uses xmlsitemap_file_transfer(), so actually loading /sitemap.xml is broken when using the previous patch.

I think the maning of xmlsitemap_file_transfer() is just coincidence and it was not intended as an implementation of hook_file_transfer().
Attached is a new patch which just renames the method.

Obviously it would be better to encapsulate this code in controller and/or service classes instead of leaving them hanging around in functions in the module file, but for now this fix is sufficient.

Cyberwolf’s picture

Cyberwolf CreditAttribution: Cyberwolf at 2DotsTwice bvba commented
Status: Active » Needs review
kalpaitch’s picture

kalpaitch CreditAttribution: kalpaitch as a volunteer and commented
Title: xmlsitemap_file_transfer causes fatal errors » xmlsitemap_file_transfer coincides with hook name provided by file_entity
kalpaitch’s picture

kalpaitch CreditAttribution: kalpaitch as a volunteer and commented

Yes that does seem exactly right @Cyberwolf.

Patch looks good. And works equally nicely.

kalpaitch’s picture

kalpaitch CreditAttribution: kalpaitch as a volunteer and commented
Status: Needs review » Reviewed & tested by the community
renatog’s picture

renatog
Location Campinas
CreditAttribution: renatog at CI&T commented

Makes sense.

renatog’s picture

renatog
Location Campinas
CreditAttribution: renatog at CI&T commented
Status: Reviewed & tested by the community » Fixed

Fixed.

Committed to the dev branch.

Thank you very much guys.

Regards.

Status: Fixed » Closed (fixed)

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