From 876103a1324369f49c931da89c7342caf7045752 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 4 Mar 2025 11:36:26 +0100 Subject: [PATCH 1/3] Prevent cross origin image requests in image cropper If we check image orientation, cropperjs makes a ajax request without propper CORS headers which prevents the browser from loading the image. According [1] to the docs we need to turn off this check in order to prevent CORS issues. [1](https://github.com/fengyuanchen/cropperjs/blob/v1/README.md#checkorientation) (cherry picked from commit 9677d43b06d514d236a4702222ba2aee53fae4e0) --- app/javascript/alchemy_admin/image_cropper.js | 8 ++++-- .../alchemy_admin/image_cropper.spec.js | 28 +++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 spec/javascript/alchemy_admin/image_cropper.spec.js diff --git a/app/javascript/alchemy_admin/image_cropper.js b/app/javascript/alchemy_admin/image_cropper.js index 3466e2875f..ad7cb14080 100644 --- a/app/javascript/alchemy_admin/image_cropper.js +++ b/app/javascript/alchemy_admin/image_cropper.js @@ -22,9 +22,11 @@ export default class ImageCropper { this.#cropSizeField = document.getElementById(formFieldIds[1]) this.elementId = elementId this.dialog = Alchemy.currentDialog() - this.dialog.options.closed = () => this.destroy() + if (this.dialog) { + this.dialog.options.closed = () => this.destroy() + this.bind() + } this.init() - this.bind() } get cropperOptions() { @@ -32,6 +34,8 @@ export default class ImageCropper { aspectRatio: this.aspectRatio, viewMode: 1, zoomable: false, + checkCrossOrigin: false, // Prevent CORS issues + checkOrientation: false, // Prevent loading the image via AJAX which can cause CORS issues minCropBoxWidth: this.minSize && this.minSize[0], minCropBoxHeight: this.minSize && this.minSize[1], ready: (event) => { diff --git a/spec/javascript/alchemy_admin/image_cropper.spec.js b/spec/javascript/alchemy_admin/image_cropper.spec.js new file mode 100644 index 0000000000..97794a5eae --- /dev/null +++ b/spec/javascript/alchemy_admin/image_cropper.spec.js @@ -0,0 +1,28 @@ +import ImageCropper from "alchemy_admin/image_cropper" + +describe("ImageCropper", () => { + describe("cropperOptions", () => { + beforeEach(() => { + document.body.innerHTML = ` +
+ + +
+ ` + Alchemy.currentDialog = jest.fn() + }) + + it("prevents CORS issues", () => { + const image = new Image() + const cropper = new ImageCropper( + image, + {}, + 1, + ["crop_from", "crop_size"], + "element_id" + ) + expect(cropper.cropperOptions["checkCrossOrigin"]).toBe(false) + expect(cropper.cropperOptions["checkOrientation"]).toBe(false) + }) + }) +}) From 8efd1421814bc290ad6c9da2d48c0c07bed5b9bb Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Wed, 5 Mar 2025 11:08:22 +0100 Subject: [PATCH 2/3] Fix restoring the cropped image on re-crop CropperJS does not support setting the min width and a crop area on init. Since a min width is a debatable feature anyway, we just remove it and let the user decide how small the image might get. (cherry picked from commit 70a07b31f38956000dbaa01a8405760446e6077f) --- app/javascript/alchemy_admin/image_cropper.js | 17 ++--------- app/views/alchemy/admin/crop.html.erb | 1 - .../alchemy_admin/image_cropper.spec.js | 30 +++++++++++++++++++ 3 files changed, 32 insertions(+), 16 deletions(-) diff --git a/app/javascript/alchemy_admin/image_cropper.js b/app/javascript/alchemy_admin/image_cropper.js index ad7cb14080..ee635c071a 100644 --- a/app/javascript/alchemy_admin/image_cropper.js +++ b/app/javascript/alchemy_admin/image_cropper.js @@ -6,16 +6,8 @@ export default class ImageCropper { #cropFromField = null #cropSizeField = null - constructor( - image, - minSize, - defaultBox, - aspectRatio, - formFieldIds, - elementId - ) { + constructor(image, defaultBox, aspectRatio, formFieldIds, elementId) { this.image = image - this.minSize = minSize this.defaultBox = defaultBox this.aspectRatio = aspectRatio this.#cropFromField = document.getElementById(formFieldIds[0]) @@ -36,12 +28,7 @@ export default class ImageCropper { zoomable: false, checkCrossOrigin: false, // Prevent CORS issues checkOrientation: false, // Prevent loading the image via AJAX which can cause CORS issues - minCropBoxWidth: this.minSize && this.minSize[0], - minCropBoxHeight: this.minSize && this.minSize[1], - ready: (event) => { - const cropper = event.target.cropper - cropper.setData(this.box) - }, + data: this.box, cropend: () => { const data = this.#cropper.getData(true) this.update(data) diff --git a/app/views/alchemy/admin/crop.html.erb b/app/views/alchemy/admin/crop.html.erb index a1a6b2a36e..fb9ab17043 100644 --- a/app/views/alchemy/admin/crop.html.erb +++ b/app/views/alchemy/admin/crop.html.erb @@ -26,7 +26,6 @@ new ImageLoader(image); new ImageCropper( image, - <%= @settings[:min_size].to_json %>, <%= @settings[:default_box].to_json %>, <%= @settings[:ratio] %>, [ diff --git a/spec/javascript/alchemy_admin/image_cropper.spec.js b/spec/javascript/alchemy_admin/image_cropper.spec.js index 97794a5eae..d0af9b9cb8 100644 --- a/spec/javascript/alchemy_admin/image_cropper.spec.js +++ b/spec/javascript/alchemy_admin/image_cropper.spec.js @@ -12,6 +12,36 @@ describe("ImageCropper", () => { Alchemy.currentDialog = jest.fn() }) + it("is sets initial data", () => { + const image = new Image() + const cropper = new ImageCropper( + image, + {}, + 1, + ["crop_from", "crop_size"], + "element_id" + ) + expect(cropper.cropperOptions["data"]).toEqual({ + height: 480, + width: 1200, + x: 0, + y: 423 + }) + }) + + it("does not set min crop size", () => { + const image = new Image() + const cropper = new ImageCropper( + image, + {}, + 1, + ["crop_from", "crop_size"], + "element_id" + ) + expect(cropper.cropperOptions["minCropBoxWidth"]).toBeUndefined() + expect(cropper.cropperOptions["minCropBoxHeight"]).toBeUndefined() + }) + it("prevents CORS issues", () => { const image = new Image() const cropper = new ImageCropper( From d67f6b190ee2f1b8fea0ee611aac77b8950845f1 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Wed, 5 Mar 2025 14:44:33 +0100 Subject: [PATCH 3/3] CI: Fix build_test if conditions syntax GitHub changed how the `failure()` and `success()` function work without any warning. Thanks for that, folks! Using job outputs instead. (cherry picked from commit cd2def920879b24ef206a0fbc87b439c9d27f833) --- .github/workflows/build_test.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build_test.yml b/.github/workflows/build_test.yml index 77b7f94644..6797a9f0e0 100644 --- a/.github/workflows/build_test.yml +++ b/.github/workflows/build_test.yml @@ -34,7 +34,6 @@ jobs: runs-on: ubuntu-22.04 name: Build JS packages needs: check_bun_lock - if: needs.check_bun_lock.outputs.bun_lock_changed == 'true' steps: - uses: actions/checkout@v4 - name: Setup Bun @@ -49,8 +48,10 @@ jobs: - name: Install dependencies run: bun install - name: bun build + if: needs.check_bun_lock.outputs.bun_lock_changed == 'true' run: bun run --bun build - uses: actions/upload-artifact@v4 + if: needs.check_bun_lock.outputs.bun_lock_changed == 'true' with: name: javascript-bundles path: vendor/javascript @@ -59,7 +60,6 @@ jobs: permissions: contents: read needs: [check_bun_lock, build_javascript] - if: ${{ success('check_bun_lock') && !failure('build_javascript') }} runs-on: ubuntu-22.04 strategy: fail-fast: false