I'm seeing s3 urls being constructed with an http scheme despite my pages being served via https. I don't have "Always serve files from S3 via HTTPS" selected because the module is meant to auto-detect the scheme. It appears the constructor for S3fsStream uses the old $is_https global and needs to be updated to be in compliance with this change record: https://www.drupal.org/node/1983438
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | interdiff-2-3.txt | 587 bytes | azinck |
| #3 | s3fs-https_not_detected-3180682-3.patch | 670 bytes | azinck |
| #2 | s3fs-https_not_detected-3180682-2.patch | 669 bytes | azinck |
Comments
Comment #2
azinck commentedComment #3
azinck commentedWhoops...need to fix the reference to global \Drupal class...
Comment #4
darvanenThat will do the job.
It would be nice to see a patch where it's injected as a dependency instead of side-loaded since it's a brand new service here, would require setting up injection for this class.
Comment #5
azinck commentedYes, I wasn’t sure if wider refactoring would be welcome here since I see other global services referenced in this class.
Comment #6
darvanenIt's a tricky one, and I can't answer that because I'm not a maintainer. Perhaps provide it as an option?
I'm considering posting a refactor of the class but there are so many outstanding issues right now that seems counter-productive.
Maintainer feedback welcome!
Comment #7
cmlaraI've spent the past couple weeks familiarizing myself with the current code base and working to close out open issue tickets. I still have some work left to do on those, but getting closer to resolving the backlog of issues. I just obtained commit access today so I am beginning to move the patches into the repository.
I intend to commit this patch as written so we can move forward, however I believe your right that we need to refactor for the newer standards of Drupal. Any help you wish to provide going forward would be appreciated.
Comment #9
cmlara