fix: URL generation when cdnUrl config option is provided

Added tests to ensure the files are accessible via generated URL

Fixes: 2

.github/workflows/checks.yml

@@ -94,6 +94,7 @@ jobs:
  S3_ACCESS_SECRET: ${{ secrets.DO_ACCESS_SECRET }}
  S3_ENDPOINT: https://sgp1.digitaloceanspaces.com
  S3_REGION: sgp1
+ S3_CDN_URL: https://testing-flydrive.sgp1.cdn.digitaloceanspaces.com
  tests-r2:
  runs-on: ${{ matrix.os }}
  concurrency:
@@ -119,4 +120,5 @@ jobs:
  S3_ACCESS_KEY: ${{ secrets.R2_ACCESS_KEY }}
  S3_ACCESS_SECRET: ${{ secrets.R2_ACCESS_SECRET }}
  S3_ENDPOINT: https://b7d56a259a224b185a70dd6e6f77d9c3.r2.cloudflarestorage.com
+ S3_CDN_URL: https://pub-7bacaefbafa643faa5799c5bf17a5b3d.r2.dev
  S3_REGION: auto

drivers/s3/driver.ts

@@ -403,14 +403,15 @@ export class S3Driver implements DriverContract {
  * Make sure using CDN URL property
  */
  if (this.options.cdnUrl) {
- return new URL(`/${this.options.bucket}/${key}`, this.options.cdnUrl).toString()
+ return new URL(key, this.options.cdnUrl).toString()
  }
 
  /**
  * Use endpoint from the config (when available)
  */
  if (this.#client.config.endpoint) {
  const endpoint = await this.#client.config.endpoint()
+
  return new URL(
  `/${this.options.bucket}/${key}`,
  `${endpoint.protocol}//${endpoint.hostname}`

package.json

@@ -5,13 +5,13 @@
  "engines": {
  "node": ">=20.6.0"
  },
+ "main": "build/index.js",
  "type": "module",
  "files": [
  "build",
  "!build/bin",
  "!build/tests"
  ],
- "main": "build/index.js",
  "exports": {
  ".": "./build/index.js",
  "./types": "./build/src/types.js",
@@ -36,21 +36,6 @@
  "version": "npm run build",
  "prepublishOnly": "npm run build"
  },
- "keywords": [
- "filesystem",
- "flydrive",
- "s3",
- "gcs",
- "r2"
- ],
- "author": "virk,flydrive",
- "license": "MIT",
- "dependencies": {
- "@humanwhocodes/retry": "^0.3.0",
- "@poppinss/utils": "^6.7.3",
- "etag": "^1.8.1",
- "mime-types": "^2.1.35"
- },
  "devDependencies": {
  "@adonisjs/env": "^6.1.0",
  "@adonisjs/eslint-config": "^1.3.0",
@@ -73,13 +58,20 @@
  "del-cli": "^5.1.0",
  "eslint": "^8.57.0",
  "get-stream": "^9.0.1",
+ "got": "^14.4.2",
  "np": "^10.0.7",
  "prettier": "^3.3.3",
  "sinon": "^18.0.0",
  "ts-node": "^10.9.2",
  "tsup": "^8.2.4",
  "typescript": "^5.5.4"
  },
+ "dependencies": {
+ "@humanwhocodes/retry": "^0.3.0",
+ "@poppinss/utils": "^6.7.3",
+ "etag": "^1.8.1",
+ "mime-types": "^2.1.35"
+ },
  "peerDependencies": {
  "@aws-sdk/client-s3": "^3.577.0",
  "@aws-sdk/s3-request-presigner": "^3.577.0",
@@ -96,6 +88,27 @@
  "optional": true
  }
  },
+ "author": "virk,flydrive",
+ "license": "MIT",
+ "homepage": "https://github.com/flydrive-js/core#readme",
+ "repository": {
+ "type": "git",
+ "url": "git+https://github.com/flydrive-js/core.git"
+ },
+ "bugs": {
+ "url": "https://github.com/flydrive-js/core/issues"
+ },
+ "keywords": [
+ "filesystem",
+ "flydrive",
+ "s3",
+ "gcs",
+ "r2"
+ ],
+ "eslintConfig": {
+ "extends": "@adonisjs/eslint-config/package"
+ },
+ "prettier": "@adonisjs/prettier-config",
  "publishConfig": {
  "access": "public",
  "tag": "latest"
@@ -115,10 +128,6 @@
  "tests/**"
  ]
  },
- "eslintConfig": {
- "extends": "@adonisjs/eslint-config/package"
- },
- "prettier": "@adonisjs/prettier-config",
  "tsup": {
  "entry": [
  "./index.ts",
@@ -136,13 +145,5 @@
  "dts": false,
  "sourcemap": true,
  "target": "esnext"
- },
- "repository": {
- "type": "git",
- "url": "git+https://github.com/flydrive-js/core.git"
- },
- "bugs": {
- "url": "https://github.com/flydrive-js/core/issues"
- },
- "homepage": "https://github.com/flydrive-js/core#readme"
+ }
 }

tests/drivers/gcs/url_generation.spec.ts

@@ -7,6 +7,7 @@
  * file that was distributed with this source code.
  */
 
+import got from 'got'
 import { test } from '@japa/runner'
 import string from '@poppinss/utils/string'
 import { Storage } from '@google-cloud/storage'
@@ -43,8 +44,13 @@ test.group('GCS Driver | getUrl', (group) => {
  usingUniformAcl: true,
  })
 
+ await fdgcs.put(key, 'hello world')
+
  const fileURL = await fdgcs.getUrl(key)
  assert.equal(fileURL, `https://storage.googleapis.com/${GCS_BUCKET}/${key}`)
+
+ const fileContents = await got.get(fileURL)
+ assert.equal(fileContents.body, 'hello world')
  })
 
  test('use custom implementation for generating public URL', async ({ assert }) => {
@@ -86,10 +92,15 @@ test.group('GCS Driver | getSignedUrl', (group) => {
  usingUniformAcl: true,
  })
 
+ await fdgcs.put(key, 'hello world')
+
  const fileURL = new URL(await fdgcs.getSignedUrl(key))
  assert.equal(fileURL.pathname, `/${GCS_BUCKET}/${key}`)
  assert.isTrue(fileURL.searchParams.has('Signature'))
  assert.isTrue(fileURL.searchParams.has('Expires'))
+
+ const fileContents = await got.get(fileURL)
+ assert.equal(fileContents.body, 'hello world')
  })
 
  test('define content type for the file', async ({ assert }) => {

tests/drivers/s3/env.ts

@@ -16,12 +16,13 @@ const env = await Env.create(new URL('../../../', import.meta.url), {
  S3_ACCESS_SECRET: Env.schema.string(),
  S3_ENDPOINT: Env.schema.string(),
  S3_REGION: Env.schema.string(),
+ S3_CDN_URL: Env.schema.string(),
 })
 
-const s3Service = env.get('S3_SERVICE')
-
-export const SUPPORTS_ACL = s3Service !== 'r2'
+export const S3_SERVICE = env.get('S3_SERVICE')
+export const SUPPORTS_ACL = S3_SERVICE !== 'r2'
 export const S3_BUCKET = env.get('S3_BUCKET')
+export const S3_CDN_URL = env.get('S3_CDN_URL')
 export const S3_REGION = env.get('S3_REGION')
 export const S3_ENDPOINT = env.get('S3_ENDPOINT')
 export const AWS_ACCESS_KEY = env.get('S3_ACCESS_KEY')

tests/drivers/s3/url_generation.spec.ts

@@ -7,6 +7,7 @@
  * file that was distributed with this source code.
  */
 
+import got from 'got'
 import { test } from '@japa/runner'
 import string from '@poppinss/utils/string'
 import { GetObjectCommand, S3Client } from '@aws-sdk/client-s3'
@@ -15,6 +16,8 @@ import { S3Driver } from '../../../drivers/s3/driver.js'
 import {
  S3_REGION,
  S3_BUCKET,
+ S3_CDN_URL,
+ S3_SERVICE,
  S3_ENDPOINT,
  SUPPORTS_ACL,
  AWS_ACCESS_KEY,
@@ -53,8 +56,18 @@ test.group('S3 Driver | getUrl', (group) => {
  supportsACL: SUPPORTS_ACL,
  })
 
+ await s3fs.put(key, 'hello world')
  const fileURL = await s3fs.getUrl(key)
+
  assert.equal(fileURL, `${S3_ENDPOINT}/${S3_BUCKET}/${key}`)
+
+ /**
+ * R2 files are private unless a cdnURL is assigned to them
+ */
+ if (S3_SERVICE !== 'r2') {
+ const fileContents = await got.get(fileURL)
+ assert.equal(fileContents.body, 'hello world')
+ }
  })
 
  test('use custom implementation for generating public URL', async ({ assert }) => {
@@ -67,7 +80,7 @@ test.group('S3 Driver | getUrl', (group) => {
  supportsACL: SUPPORTS_ACL,
  urlBuilder: {
  async generateURL(fileKey, fileBucket) {
- return `https://cdn.example.com/${fileBucket}/${fileKey}`
+ return new URL(fileKey, `https://cdn.example.com/${fileBucket}/`).toString()
  },
  },
  })
@@ -84,18 +97,23 @@ test.group('S3 Driver | getUrl', (group) => {
  client: client,
  bucket: S3_BUCKET,
  supportsACL: SUPPORTS_ACL,
- cdnUrl: 'https://cdn.example.com',
+ cdnUrl: S3_CDN_URL,
  })
 
+ await s3fs.put(key, 'hello world')
  const fileURL = await s3fs.getUrl(key)
- assert.equal(fileURL, `https://cdn.example.com/${S3_BUCKET}/${key}`)
+
+ const fileContents = await got.get(fileURL)
+
+ assert.equal(fileURL, new URL(key, S3_CDN_URL).toString())
+ assert.equal(fileContents.body, 'hello world')
  })
 })
 
 test.group('S3 Driver | getSignedUrl', (group) => {
  group.each.setup(() => {
  return async () => {
- await deleteS3Objects(client, S3_BUCKET, '/')
+ // await deleteS3Objects(client, S3_BUCKET, '/')
  }
  })
  group.each.timeout(10_000)
@@ -104,17 +122,23 @@ test.group('S3 Driver | getSignedUrl', (group) => {
  const key = `${string.random(6)}.txt`
 
  const s3fs = new S3Driver({
- visibility: 'public',
+ visibility: 'private',
  client: client,
  bucket: S3_BUCKET,
  supportsACL: SUPPORTS_ACL,
  })
 
+ await s3fs.put(key, 'hello world')
+
  const fileURL = new URL(await s3fs.getSignedUrl(key))
+ const fileContents = await got.get(fileURL)
+
  assert.include(fileURL.hostname, S3_BUCKET)
  assert.equal(fileURL.pathname, `/${key}`)
  assert.isTrue(fileURL.searchParams.has('X-Amz-Signature'))
  assert.isTrue(fileURL.searchParams.has('X-Amz-Expires'))
+
+ assert.equal(fileContents.body, 'hello world')
  })
 
  test('define content type for the file', async ({ assert }) => {