Functionality not working for 2.1.3 or 2.1.4

Created on 16 February 2024, 10 months ago

Problem/Motivation

We have a DDEV powered project set up with stage_file_proxy, and the functionality was working while using version 2.1.1.

Upgrade to 2.1.3, the functionality breaks.

2.1.4, still broken.

Revert to 2.1.2, fixes it.

Not entirely sure why it would break, but here we are. The behavior is as if the module wasn't enabled at all. Normally we override the config with a settings file using $config, but that doesn't seem to work, and filling out the configuration form in the CMS does nothing as well.

Looking for some clues on how to debug, in case maybe something is conflicting, otherwise we are frozen at version 2.1.2 until we can figure this out.

Tried the 2.1.x-dev branch too, and it did not help.

🐛 Bug report
Status

Active

Version

2.1

Component

Code

Created by

🇺🇸United States nessthehero

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @nessthehero
  • 🇺🇸United States nessthehero

    I've noticed that the itok key in the request on my localhost is different than if I went directly to the source environment and found the same image.

    Not sure if related. If I update the local proxy redirect to use the same itok as the source environment, the image request works.

  • 🇳🇱Netherlands eelkeblok Netherlands 🇳🇱

    The itok is environment-specific; it is a safety-mechanism to indicate to the style subsystem that the request originates from an actual generated URL for a style (as in, there is an actual web page that needs this derived image), as opposed to an attempt from the requesting party at generating as many derived images as possible. I am uncertain how SFP deals with itoks for the parent environment, but my guess is, it doesn't (the itok, by nature, it not guessable from the outside, so it would have some sort of trusted channel to the parent). If you have SFP configured to not download the root image, but download derived images instead, I would expect it needs the parent environment to already have generated the derived image, or it will fail (due to local environment not knowing the correct itok; if it has already been generated, the itok is no longer relevant, it is then the web server serving the image and it doesn't check the itok).

    Setting to support request for now. You could also consider it "Maintainer needs more info", if you still believe it to be a bug.

  • 🇳🇱Netherlands eelkeblok Netherlands 🇳🇱
  • 🇺🇸United States m-simmons

    I also have this issue with local DDEV environment. The interim solution of reverting to 2.1.2 works for me as well.

  • 🇺🇸United States m-simmons

    I also have this issue with local DDEV environment. The interim solution of reverting to 2.1.2 works for me as well.

  • 🇧🇪Belgium xaa Brussels

    same issue here (not using ddev).
    2.1.2 works fine as well.

  • Status changed to Postponed: needs info 8 months ago
  • 🇺🇸United States greggles Denver, Colorado, USA

    Version 2.1.4 on core 10.2.3 works fine for me with my settings.

    For folks encountering problems can you provide more details like what your settings are?

    If there is any error, what error do you experience?

  • Status changed to Active 8 months ago
  • 🇺🇸United States paulmckibben Atlanta, GA

    I'm also experiencing this issue on a DDEV environment. I _think_ the problem may be cache-related. Invoking drush cr results in loading the images successfully.

    Browser: Chrome version 124.0.6367.78
    OS: MacOS Monterey 12.7.4

    Local DDEV:
    Drupal core: 10.2.5
    PHP: 8.2
    stage_file_proxy version: 2.1.4

    Errors when attempting to load an image:
    Repeating 302 redirects to the same local URL until it stops trying.
    Each one shows a cache hit:
    X-Drupal-Cache: HIT
    X-Frame-Options: SAMEORIGIN

    As mentioned above, after drush cr, the problem clears up for the affected images.

  • 🇺🇸United States paulmckibben Atlanta, GA

    Workaround for this problem is to click "do not cache markup" in development settings.
    It will still fail on the first page load, but if you reload the page, the images will load.

  • 🇺🇸United States greggles Denver, Colorado, USA

    Thanks for those details, @paulmckibben! I didn't try to reproduce just yet.

    I wonder what your process is for enabling stage_file_proxy in these development environments?

    For me I have hooks in .ddev/config.yaml:

    hooks:
        post-import-db:
            - exec: ./vendor/bin/robo run:update-database
            - exec: ./vendor/bin/robo run:enable-dev-modules 1
    

    Those Robo tasks enable stage_file_proxy and run a cache rebuild. I'm not sure what can be fixed in an appropriate way in stage_file_proxy to do a cache rebuild, but I'm open to ideas of how to fix this if as a "bug" in stage file proxy if there's a proposal.

  • 🇺🇸United States paulmckibben Atlanta, GA

    Thanks, @greggles.

    I enable stage_file_proxy manually, when I need it, i.e. `ddev drush en stage_file_proxy`, and then use `ddev drush config-set` to set the origin URL.

    I'm curious if that makes a difference. What I see right now is, if an image does not exist in the file system and results in a 404, stage_file_proxy gives us a 302 redirect to the same URL. The 404s repeat, and so do the redirects, until the browser decides it's too many. Those repeated redirects happen pretty quickly, in a split second.

    I think it's either the delay in downloading the image from the origin, or a delay in the container's file system, that makes it appear as if the image failed to download, and you still get a broken image.

    With "do not cache markup" checked in the development settings, if I reload the page, by that time the image has finished downloading and exists in the container's file system, so it's no longer broken.

    I took a look at the code and don't have any good ideas for a fix yet. I'm not sure if introducing a delay is a good idea. And the redirect itself should not be cached if I'm reading the code right. I did see the redirect logic was introduced in 2.1.3, so that lends credence to the correlation between the redirect and this problem.

  • 🇺🇸United States greggles Denver, Colorado, USA

    I wonder if you could do `ddev drush cr` right after the config-set (perhaps scripted) to get things working?

    The title here states this is a regression after 2.1.2. Is it a regression for you, @paulmckibben? If you switch back to the 2.1.2 version does the problem go away?

  • 🇺🇸United States paulmckibben Atlanta, GA

    Thanks, @greggles. I did the following to answer your questions.

    Test setup in my local ddev environment:

    Test #1: using version 2.1.4, does invoking `ddev drush cr` immediately after enabling stage_file_proxy and setting the origin help?
    Result: no, it does not help. However, invoking `ddev drush cr` after attempting to load a page with stage_file_proxy enabled, and hard-refreshing the page afterward, does help.

    1. Perform the Test Setup listed above.
    2. Load the homepage in a browser using incognito mode. Note missing images.
    3. Enable stage_file_proxy using drush: `ddev drush en stage_file_proxy`
    4. Set the origin: `ddev drush cset stage_file_proxy.settings origin https://my-origin.example.com`
    5. `ddev drush cr`
    6. Hard-refresh the browser to reload the homepage. The images still do not load.
    7. Hard-refresh the browser again. Still missing the same images.
    8. `ddev drush cr`
    9. Hard-refresh the browser again. Now the images appear.

    Test #2: does the problem occur using version 2.1.2?
    Result: No.

    1. Disable stage_file_proxy 2.1.4 and remove from codebase
    2. Install in codebase, but do not enable, stage_file_proxy 2.1.2
    3. Perform the test setup listed above.
    4. Load the homepage in a browser in incognito mode. Note missing images.
    5. Enable stage_file_proxy using drush: `ddev drush en stage_file_proxy`
    6. Set the origin: `ddev drush cset stage_file_proxy.settings origin https://my-origin.example.com`
    7. Hard-refresh the browser to reload the homepage. The images load as expected.
  • 🇺🇸United States greggles Denver, Colorado, USA

    The research is SUPER helpful. Thanks paulmckibben!

  • 🇦🇺Australia richard.thomas

    We're seeing what I think is it the same issue of an infinite redirect loop when using the non-hotlinking mode and accessing image style derivatives, I believe the issue is caused by this change (and requires having the page_cache module enabled):

    https://www.drupal.org/project/stage_file_proxy/issues/3396634 🐛 CORS headers aren't applied to the module response Fixed

    After StageFileProxySubscriber has fetched the file, it returns a RedirectResponse (to the same URL as the current request) which shouldn't be cacheable. However \Drupal\Core\EventSubscriber\RedirectResponseSubscriber has logic that converts it into a LocalRedirectResponse which is cacheable. So the redirect to the same URL gets cached by page_cache and the browser gets into an infinite loop.

    I think this is only an issue for image style derivatives as for other public files, once the file is downloaded the web server would intercept the next request so it wouldn't get to drupal to hit the page cache.

    Using the page_cache kill switch seems to resolve the issue for me locally. I'll create an issue fork with the fix.

  • Pipeline finished with Success
    7 months ago
    Total: 141s
    #169265
  • Status changed to Needs review 7 months ago
  • 🇦🇺Australia richard.thomas

    Created an MR with the page cache kill switch, works in my local testing to prevent the infinite redirects on image style derivative URLs.

  • Pipeline finished with Success
    7 months ago
    Total: 143s
    #171248
  • Status changed to Needs work 7 months ago
  • 🇺🇸United States paulmckibben Atlanta, GA

    @richard.thomas Thanks for the patch! I tried it out, and though it seems to keep the redirect from being cached, I now see a different problem with image style derivatives: as an anonymous user, when I load a page that uses image style derivative images, they return 404, and the 404 response gets cached by Drupal's page cache. I see X-Drupal-Cache: HIT among the 404 response headers when the browser requests the image derivative URL.

    Checking the file system, the original image has been downloaded, and I can see it in the file system. However, I don't see the image style derivative in the file system. Since Drupal has cached both the page URL and the image derivative URL, subsequent page reloads do not attempt to recreate the derivative image.

    Note: if I click "do not cache markup" in the development settings, then the behavior is slightly different: on the first page load, the derivative images are 404. With a normal page refresh, the derivative images load. So part of the problem may be that derivative images are not generated if stage_file_proxy has to retrieve the original file from the origin server. I believe this is still a regression.

    This is definitely an improvement, but we still need to figure out how to fix image style derivatives.

  • 🇦🇺Australia richard.thomas

    Afraid I can't see that behaviour (404s until cache clear) here in my local testing, what's your config for stage_file_proxy.settings? The other possibility if it works after cache clears/disabling markup cache is that somehow the image token "itok" values are invalid on first load? If that's the case then the image style controller would return a 404 error that is intended to be cached.

  • 🇺🇸United States smustgrave

    Still will backup 2.1.x but lets try and land in 3.0.x first.

  • 🇺🇸United States smustgrave

    Putting back into review for #20

  • First commit to issue fork.
  • 🇳🇱Netherlands spokje

    spokje changed the visibility of the branch 2.1.x to hidden.

  • 🇳🇱Netherlands spokje

    spokje changed the visibility of the branch 3.0.x to hidden.

  • 🇳🇱Netherlands spokje

    spokje changed the visibility of the branch 3.1.x to hidden.

  • Pipeline finished with Failed
    about 1 month ago
    Total: 153s
    #335354
  • Pipeline finished with Failed
    about 1 month ago
    Total: 153s
    #335355
  • Pipeline finished with Failed
    about 1 month ago
    Total: 154s
    #335353
  • Pipeline finished with Success
    about 1 month ago
    Total: 1255s
    #335359
  • 🇳🇱Netherlands spokje

    Seeing the same behaviour as #16, rerolled MR!69 for 3.1.x-dev

    • smustgrave committed 4c69e0c3 on 3.1.x
      Issue #3421948 by spokje, richard.thomas, paulmckibben, greggles,...
    • smustgrave committed 7f0861ca on 3.0.x
      Issue #3421948 by spokje, richard.thomas, paulmckibben, greggles,...
  • 🇳🇱Netherlands cleverhoods

    cleverhoods changed the visibility of the branch 3.1.x to active.

  • Status changed to Fixed 9 days ago
  • 🇳🇱Netherlands cleverhoods

    cleverhoods changed the visibility of the branch 3.1.x to active.

Production build 0.71.5 2024