- Issue created by @Driskell
- Status changed to Needs review
7 months ago 9:31am 2 May 2024 - π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
π Since 4.x release, hook_file_url_alter is no longer called, breaking several modules Active was fixed and will ship in the
4.1
release. Can you please test again and confirm that the problem is gone? π - Status changed to Postponed: needs info
7 months ago 9:05am 9 May 2024 - π¬π§United Kingdom Driskell
@wim-leers Thanks! CDN now calls the URL alter - so that's one step forwards.
However that's the first part and as you noted in #3391254 the images won't be converted to CDN urls:
β¦ I do see one major problem:
[...]
β¦ that would mean that "cropped image" file URLs never actually would have been served from the CDN. But until #3179753: Improve far-future support: generate dynamically generated files automatically (f.e. image style derivatives) is supported, that wouldn't be possible either.
I think though this can be now easily worked on here. I can't see a specific reason why the crop module should change the URL to an external one. I can see in the comments in the code it's relating to the query string getting converted somewhere - so could be a workaround for some other issue that needs fixing first too.
// If the URI has a schema and that is not http, https or data, convert // the URI to the external URL. Otherwise the appended query argument // will be encoded. // @see file_create_url()
I'll investigate this to see if perhaps it is an old issue that was already fixed, or if it needs a fix, and report back when I can. I'm not the maintainer but postponed until that info is available is way forwards.
- π¬π§United Kingdom Driskell
I think this still requires CDN change - as Crop has to add a query string - but core stream wrapper getExternalUrl actually encodes the path including query string so the query becomes part of the final path part public://styles/file.png%3Dquery which then breaks the final path. The image style module builds its URL by converting to absolute first then adding `itok`. It feels like a big core change to support query on stream wrapper paths. So I don't think Crop is doing anything wrong here.
I think CDN needs to check after the file_url_alter in canServe for the current base URI and support that as a valid path to convert to CDN. Otherwise I can't see this working well without Crop being "CDN-aware" which not sure it should be.
- πΊπΈUnited States Harish.04
Hi @driskell / @wim-leers,
Is there a separate ticket raised for tracking this issue ? we are facing similar issue with CDN recently, In our custom module we are using hook_file_url_alter for processing some of the file/Image urls for one of the sub site but recently we noticed that hook_file_url_alter function is not getting called when we render those images from CDN domain. I upgraded to CDN to 4.1.0 as well but still the issue seems to exist.
Any help would be appreciated.
Thanks,
Harish M. - π¬π§United Kingdom Driskell
@Harish.04
Latest version does see file_url_alter get called. It calls at the point that the CDN URL is generated rather than when the image is pulled from the CDN.
I did raise this issue on CDN now though but I think it's not your issue as the hook does run, it's just CDN refuses to handle an absolute URL: https://www.drupal.org/project/cdn/issues/3453869 β¨ Support absolute URLs that point to public stream wrapper files in FileUrlGenerator Active
- πΊπΈUnited States Harish.04
Hi @driskell,
yeah they have mentioned hook_file_url_alter is restored in CDN 4.1.0 version but doesn't seem to be fixing our issue. In our case we are altering the file path in hook_file_url_alter in our custom module (basically stripping our sub domain prefix from base url) but it doesn't seem to be working.
It calls at the point that the CDN URL is generated rather than when the image is pulled from the CDN. - so the custom code written for hook_file_url_alter doesn't get called when image is pulled from the CDN. - π¬π§United Kingdom Driskell
@Harish.04 I'm not sure I can help very much but the hook won't receive the absolute URL unless some other hook has expanded it. For an image it would receive public://2024-05/file.jpg as an example. So you can't really strip a prefix. If you're looking to alter what CDN is doing then I think you need to use a different method as CDN is not using hook_file_url_alter for a long time and instead overrides the FileUrlGenerator. Maybe previous in much older versions it did and in that case you may have seen the CDN url pass through the hook - but that is not the case anymore by design.
- πΊπΈUnited States Harish.04
Thanks @driskell I will check on different options for overriding the File URL's.